search

code-423n4 / 2021-04-basedloans-findings

0 stars 1 forks source link

Potential reentrancy caused by the `doTransferOut` function of the `CEther` contract. #41

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

shw

Vulnerability details

Impact

The doTransferOut function in the CEther contract is implemented using a low level call (i.e., to.call.value(amount)("");) instead of the Solidity keyword, transfer, which is adopted by Compound. The change of transferring ETH could introduce potential reentrancy since call forwards all remaining gas while transfer only forwards 2300 of gas.

Proof of Concept

Referenced code: CEther.sol#L146

Tools Used

None

Recommended Mitigation Steps

Limit the forwarded gas to 2300, or add reentrancy guards to critical functions that should not be re-entered.

ghoul-sol commented 3 years ago

I understand the motivation behind the statement but I cannot find any vulnerability. One precaution used during refactoring was to move doTransferOut at the end of the function which makes reentry pointless. Second precaution is nonReentrant modifier that is used in all functions using doTransferOut internally. I don't see a bug here.

cemozerr commented 3 years ago

Re-entrancy doesn't seem possible due to the precautions @ghoul-sol mentioned, so I'm closing this as an invalid bug.