Lines of code

Vulnerability details

Impact

Every time transferFrom or transfer function in ERC20 standard is called there is a possibility that underlying smart contract did not transfer the exact amount entered.
It is required to find out contract balance increase/decrease after the transfer.
This pattern also prevents from re-entrancy attack vector.
Also make sure to use SafeERC20.
If this function is meant to be used for WETH only, then consider adding require to avoid other tokens deposits.
And finally reduce if nesting.

Proof of Concept

Tools Used

Recommended Mitigation Steps

Recommended code:

function _transferTokensToSelf(address underlyingToken, uint256 collateralInitial) internal override {
    // Convert eth to weth if received eth, otherwise transfer weth
    if (msg.value > 0) {
        if (msg.value != collateralInitial) revert IllegalArgument("msg.value doesn't match collateralInitial");
        IWETH9(wethAddress).deposit{value: msg.value}();
        return;
    }

    // reduced if nesting
    uint256 balanceBefore =  IERC20(underlyingToken).balanceOf(address(this)); // remembering asset balance before the transfer
    IERC20(underlyingToken).safeTransferFrom(msg.sender, address(this), collateralInitial); // make sure to use SafeERC20
    uint256 newCollateralInitial =  IERC20(underlyingToken).balanceOf(address(this)) - balanceBefore; // updating actual amount to the contract balance increase

    // no re-entrancy attack vector below here

    require(newCollateralInitial == collateralInitial); // making sure we received correct amount  

}  

The text was updated successfully, but these errors were encountered:

All reactions