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

6 stars 4 forks source link

Unsafe use of .transfer #167

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

User funds can be locked by using a smart contract wallet with inefficient callback, or by future hardforks that change the gas consumption.

address.transfer has been suggested to deprecate by most auditors, because of the potential OOG error. However, payable(address).transfer is still use heavily through out the codeabase.

Reference: https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

Recommended Mitigation Steps

use call instead:

(bool success, ) = msg.sender.call.value(amount)("");
require(success, "Transfer failed.");
chase-manning commented 2 years ago

Duplicate of #52