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

6 stars 4 forks source link

Avoid payable.transfer #219

Closed gzeoneth closed 2 years ago

gzeoneth commented 2 years ago

Originally submitted by warden horsefacts in https://github.com/code-423n4/2022-04-backd-findings/issues/199, duplicate of https://github.com/code-423n4/2022-04-backd-findings/issues/52.

Avoid payable.transfer

EthPool and EthVault both use payable(address).transfer to transfer ETH.

It's considered a best practice to avoid this pattern for ETH transfers, since it forwards a fixed amount of gas and may revert if future gas costs change. (See the Consensys Diligence article here).

EthPool#_doTransferOut

    function _doTransferOut(address payable to, uint256 amount) internal override {
        to.transfer(amount);
    }

EthVault#_transfer

    function _transfer(address to, uint256 amount) internal override {
        payable(to).transfer(amount);
    }

EthVault#_depositToTreasury

    function _depositToTreasury(uint256 amount) internal override {
        payable(addressProvider.getTreasury()).transfer(amount);
    }

Consider using OpenZeppelin Address.sendValue, but take care to avoid reentrancy. Callers of these internal functions should be protected with a reentrancy guard.

JeeberC4 commented 2 years ago

created required json file