search

code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

CompoundHandler and other contracts call native payable.transfer #157

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Title

CompoundHandler topUp(), EthPool _doTransferOut() and VaultReserve withdraw() call native payable.transfer

Impact

Multiple operations are handled by using a payable.transfer() call. transfer() has a gas budget limit which is unsafe because gas costs can and may change and can fail when the user is a smart contract.

Proof of Concept

CompoundHandler, VaultReserve, and EthPool have a function that calls transfer:

CompoundHandler.topUp

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/actions/topup/handlers/CompoundHandler.sol#L79

VaultReserve.withdraw

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/vault/VaultReserve.sol#L81

EthPool._doTransferOut

https://github.com/code-423n4/2022-04-backd/blob/c856714a50437cb33240a5964b63687c9876275b/backd/contracts/pool/EthPool.sol#L30

Recommended Mitigation Steps

Using low-level call.value(amount) with the corresponding result check or using the OpenZeppelin Address.sendValue, and using nonReentrant is advised

gzeoneth commented 2 years ago

Duplicate #52