search

code-423n4 / 2021-11-fei-findings

0 stars 0 forks source link

Don't cache bool `check` #143

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

0x0x0x

Vulnerability details

Concept

PegExchanger.sol#takeFrom, PegExchanger.sol#giveTo and TRIBERagequit.sol#takeFrom are implemented in a similar manner. After transferFrom call, the return value is cached to a parameter called check, after that it is controlled whether check is true. But the used transferFrom functions never return false. They simply revert if there is a problem with the transaction. Therefore, it is possible to implement those functions more gas efficient.

Therefore following implementation:

function takeFrom(address target, uint256 amount) internal {
        bool check = token0.transferFrom(target, address(this), amount);
        require(check, "erc20 transfer failed");
    }

Can be replaced with:

function takeFrom(address target, uint256 amount) internal {
        token0.transferFrom(target, address(this), amount);
    }
elee1766 commented 2 years ago

120

pauliax commented 2 years ago

Valid optimization. Making this a primary issue, as it covers both cases: inline transfer and removing the boolean check.