code-423n4 / 2023-08-goodentry-findings

3 stars 2 forks source link

Stuck tokens due to using transfer in GeVault, which only forwards 2300 gas #314

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/GeVault.sol#L232

Vulnerability details

Impact

Stuck tokens in a contract that spends more than 2300 gas on receive().

Proof of Concept

This is a known issue, see this finding for a similar description.

In the GeVault, the withdraw() function, when the token to withdraw is WETH, first unwrapps it and then transfers to the msg.sender the amount, as can be seen below:

function withdraw(uint liquidity, address token) public nonReentrant returns (uint amount) {
    ...
    if (token == address(WETH)){
      WETH.withdraw(bal);
      payable(msg.sender).transfer(bal); // TO SUBMIT, might cause frozen assets
    }
    else {
      ERC20(token).safeTransfer(msg.sender, bal);
    }
    ...
}

The vulnerability is that transfer() will only forward 2300 gas, which means that the transaction might revert due to OutOfGas if the smart contract has additional logic in the receive() function. An example of such a vulnerable smart contract can be seen below:

contract SampleVulnerableContract {
    address public owner;

    constructor (address owner_) {
        owner = owner_;
    }

    function deposit(GeVault vault_, address token_, uint256 amount_) external payable {
        vault_.deposit{value: msg.value}(token_, amount_);
    }

    function withdraw(GeVault vault_, uint256 liquidity_, address token_) external {
        vault_.withdraw(liquidity_, token_);
    }

    receive() external payable {
        payable(owner).transfer(msg.value);
    }
}

A POC was carried out in Foundry, showing that the mentioned smart contract is vulnerable to this issue. The brownie setup was migrated to Foundry.

// SPDX-License-Identifier: agpl-3.0
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";

import {ERC20} from "contracts/openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";
import {UpgradeableBeacon} from "contracts/openzeppelin-solidity/contracts/proxy/beacon/UpgradeableBeacon.sol";

import {RoeRouter} from "contracts/RoeRouter.sol";
import {OptionsPositionManager} from "contracts/PositionManager/OptionsPositionManager.sol";
import {TokenisableRange} from "contracts/TokenisableRange.sol";
import {RangeManager} from "contracts/RangeManager.sol";
import {GeVault} from "contracts/GeVault.sol";

import {ILendingPoolAddressesProvider} from "interfaces/ILendingPoolAddressesProvider.sol";
import {ILendingPoolConfigurator} from "interfaces/ILendingPoolConfigurator.sol";
import {ILendingPool} from "interfaces/IAaveLendingPoolV2.sol";
import {IUniswapV3Factory} from "interfaces/IUniswapV3Factory.sol";
import {IUniswapV3Pool} from "interfaces/IUniswapV3Pool.sol";
import {IAaveOracle} from "interfaces/IAaveOracle.sol";
import {ICreditDelegationToken} from "interfaces/ICreditDelegationToken.sol";

contract SampleVulnerableContract {
    address public owner;

    constructor (address owner_) {
        owner = owner_;
    }

    function deposit(GeVault vault_, address token_, uint256 amount_) external payable {
        vault_.deposit{value: msg.value}(token_, amount_);
    }

    function withdraw(GeVault vault_, uint256 liquidity_, address token_) external {
        vault_.withdraw(liquidity_, token_);
    }

    receive() external payable {
        payable(owner).transfer(msg.value);
    }
}

contract SwapTest is Test {
    address public constant USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;
    address public constant WETH = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
    address public constant TREASURY =
        0x50101017adf9D2d06C395471Bc3D6348589c3b97;
    address public constant ROUTER = 0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D;
    address public constant ROUTERV3 =
        0xE592427A0AEce92De3Edee1F18E0157C05861564;
    address public constant UNISWAPPOOLV3 =
        0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640;
    address public constant UNISWAPPOOLV3_3 =
        0x8ad599c3A0ff1De082011EFDDc58f1908eb6e6D8;
    address public constant UNISWAPV3_FACTORY =
        0x1F98431c8aD98523631AE4a59f267346ea31F984;
    //LPAD for btc 0xe7f6F6Cd1Be8313a05e0E38bA97B2A5Dfed7616d - for weth 0x01b76559D512Fa28aCc03630E8954405BcBB1E02
    address public constant LENDING_POOL_ADDRESSES_PROVIDER =
        0x01b76559D512Fa28aCc03630E8954405BcBB1E02;
    address public constant AAVE_WETH =
        0x030bA81f1c18d280636F32af80b9AAd02Cf0854e;
    address public constant AAVE_USDC =
        0xBcca60bB61934080951369a648Fb03DF4F96263C;
    address public constant NULL = 0x0000000000000000000000000000000000000000;
    address public constant TIMELOCK =
        0xA10feBCE203086d7A0f6E9A2FA46268Bec7E199F;

    address public constant THETA_A =
        0x78b787C1533Acfb84b8C76B7e5CFdfe80231Ea2D; // matic "0xb54240e3F2180A0E14CE405A089f600dc2D8457c"
    address public constant THETA_STBDEBT =
        0x8B6Ab2f071b27AC1eEbFfA973D957A767b15b2DB; // matic "0x92ED25161bb90eb0026e579b60B8D96eE3b7A15F"
    address public constant THETA_VARDEBT =
        0xB19Dd5DAD35af36CF2D80D1A9060f1949b11fCb0; // matic "0x51b89b9e24bc85d6756571032B8bf5660Bf6FbE5"
    address public constant THETA_100BPS_FIXED =
        0xfAdB757A7BC3031285417d7114EFD58598E21d79; // "0xEdFbbeDdc3CB3271fd60E90E184B151C76Cd88aB"

    uint128[] public TICKS = [1000, 1100, 1200, 1300, 1400, 1500];

    address public owner;
    address public user;
    ILendingPoolAddressesProvider public lpadd;
    ILendingPool public lendingPool;
    ILendingPoolConfigurator public config;
    address public poolAdmin;
    RoeRouter public roeRouter;
    OptionsPositionManager public pm;
    TokenisableRange public tr;
    address public trb;
    RangeManager public r;
    IAaveOracle public oracle;
    GeVault public geVault;

    function setUp() public {
        vm.createSelectFork("https://eth.llamarpc.com", 16360000);

        owner = makeAddr("owner");
        user = makeAddr("user");

        lpadd = ILendingPoolAddressesProvider(LENDING_POOL_ADDRESSES_PROVIDER);
        lendingPool = ILendingPool(lpadd.getLendingPool());
        poolAdmin = lpadd.getPoolAdmin();
        config = ILendingPoolConfigurator(lpadd.getLendingPoolConfigurator());
        vm.prank(poolAdmin);
        lendingPool.setSoftLiquidationThreshold(102e16);
        oracle = IAaveOracle(lpadd.getPriceOracle());

        vm.startPrank(owner);
        roeRouter = new RoeRouter(TREASURY);
        roeRouter.addPool(LENDING_POOL_ADDRESSES_PROVIDER, USDC, WETH, ROUTER);
        pm = new OptionsPositionManager(address(roeRouter));
        vm.stopPrank();

        vm.prank(TIMELOCK);
        lendingPool.PMAssign(address(pm));

        deal(WETH, owner, type(uint256).max);
        _approveAndDepositToLendingPool(owner, WETH, 30e10);

        deal(USDC, owner, type(uint256).max);
        deal(USDC, user, 6e10);
        _approveAndDepositToLendingPool(owner, USDC, 30e10);
        _approveAndDepositToLendingPool(user, USDC, 1e10);

        vm.startPrank(owner);
        tr = new TokenisableRange();
        trb = address(new UpgradeableBeacon(address(tr)));
        r = new RangeManager(lendingPool, ERC20(USDC), ERC20(WETH));
        geVault = new GeVault(
            TREASURY,
            address(roeRouter),
            UNISWAPPOOLV3,
            0,
            "GeVault WETHUSDC",
            "GEV-ETHUSDC",
            WETH,
            false
        );
        vm.stopPrank();

        vm.label(address(lendingPool), "lendingPool");
        vm.label(address(roeRouter), "roeRouter");
        vm.label(address(pm), "pm");
        vm.label(address(tr), "tr");
        vm.label(address(trb), "trb");
        vm.label(address(r), "r");
        vm.label(address(geVault), "geVault");

        deal(USDC, owner, type(uint256).max);
        deal(WETH, owner, type(uint256).max);

        address[] memory addresses_ = new address[](TICKS.length);

        vm.startPrank(owner);
        for (uint i_; i_ < TICKS.length; i_++) {
            TokenisableRange t_ = new TokenisableRange();
            t_.initProxy(
                oracle,
                ERC20(USDC),
                ERC20(WETH),
                TICKS[i_] * 1e10,
                TICKS[i_] * 10001 * 1e6,
                vm.toString(TICKS[i_]),
                vm.toString((TICKS[i_] * 10001) / 1e4),
                true
            );
            (uint256 usdAmount_, uint256 ethAmount_) = _liquidityRatio(
                TICKS[i_] * 1e4,
                TICKS[i_] * 10001
            );
            ERC20(USDC).approve(address(t_), type(uint256).max);
            ERC20(WETH).approve(address(t_), type(uint256).max);
            t_.init(usdAmount_, ethAmount_);
            addresses_[i_] = address(t_);
        }
        vm.stopPrank();

        vm.startPrank(TIMELOCK);

        oracle.setAssetSources(addresses_, addresses_);

        for (uint i_; i_ < addresses_.length; i_++) {
            string memory sym_ = TokenisableRange(addresses_[i_]).symbol();
            string memory name_ = TokenisableRange(addresses_[i_]).name();
            ILendingPoolConfigurator.InitReserveInput[]
                memory inputs_ = new ILendingPoolConfigurator.InitReserveInput[](
                    1
                );
            inputs_[0] = ILendingPoolConfigurator.InitReserveInput(
                THETA_A,
                THETA_STBDEBT,
                THETA_VARDEBT,
                18,
                THETA_100BPS_FIXED,
                addresses_[i_],
                owner,
                NULL,
                sym_,
                string(abi.encodePacked("Roe ", name_)),
                string(abi.encodePacked("roe", sym_)),
                string(abi.encodePacked("Roe variable debt bearing ", name_)),
                string(abi.encodePacked("vd", sym_)),
                string(abi.encodePacked("Roe stable debt bearing ", name_)),
                string(abi.encodePacked("sd", sym_)),
                ""
            );
            config.batchInitReserve(inputs_);
        }

        for (uint i_; i_ < addresses_.length; i_++) {
            config.configureReserveAsCollateral(
                addresses_[i_],
                9250,
                9500,
                10300
            );
            config.enableBorrowingOnReserve(addresses_[i_], true);
        }
        vm.stopPrank();

        vm.startPrank(owner);
        for (uint i_; i_ < addresses_.length; i_++) {
            TokenisableRange tr_ = TokenisableRange(addresses_[i_]);
            tr_.approve(address(lendingPool), 2 ** 256 - 1);
            lendingPool.deposit(address(tr_), tr_.balanceOf(owner), owner, 0);
            geVault.pushTick(addresses_[i_]);
        }

        geVault.rebalance();

        vm.stopPrank();
    }

    function test_POC_geVault_TransferOnly2300gas_StuckTokens() public {
        deal(user, 1e18);
        vm.startPrank(user);
        SampleVulnerableContract sv_ = new SampleVulnerableContract(user);
        sv_.deposit{value: 1e18}(geVault, WETH, 1e18);
        vm.expectRevert(); // EvmError: OutOfGas
        sv_.withdraw(geVault, 0, WETH);
        vm.stopPrank();
    }

    function _approveAndDepositToLendingPool(
        address user_,
        address asset_,
        uint256 amount_
    ) internal {
        vm.startPrank(user_);
        ERC20(asset_).approve(address(lendingPool), amount_);
        lendingPool.deposit(asset_, amount_, user_, 0);
        vm.stopPrank();
    }

    function _liquidityRatio(
        uint256 rangeLow_,
        uint256 rangeHigh_
    ) internal returns (uint256, uint256) {
        IUniswapV3Pool pool_ = IUniswapV3Pool(
            IUniswapV3Factory(UNISWAPV3_FACTORY).getPool(USDC, WETH, 3000)
        );
        (uint256 sqrtPriceX96_, , , , , , ) = pool_.slot0();
        string[] memory inputs_ = new string[](5);
        inputs_[0] = "python3";
        inputs_[1] = "test/liquidityRatio.py";
        inputs_[2] = vm.toString(rangeLow_);
        inputs_[3] = vm.toString(rangeHigh_);
        inputs_[4] = vm.toString(sqrtPriceX96_);
        string memory outputs_ = string(vm.ffi(inputs_));
        return (
            vm.parseJsonUint(outputs_, ".usdAmount"),
            vm.parseJsonUint(outputs_, ".ethAmount")
        );
    }
}

Tools Used

Vscode, Foundry

Recommended Mitigation Steps

Instead of using transfer() when withdrawing WETH, use payable(msg.sender).call{value: bal}(""), which forwards 63/64 of the gas left, allowing it to be increased if needed.

Assessed type

Payable

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #79

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Out of scope