Asymmetry contest - cloudjunky's results

Lines of code

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

Vulnerability details

Title

Protocol fails at higher maxAmounts

Code Lines Affected

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

Impact

If the protocol is configured with a single derivative of Reth and the maxDeposit is set to 2000 ETH staking fails for some deposit amounts. Derivatives can be added or subtracted at any time by the owner of the protocol.

Proof of Concept

In the proof of concept code below setMaxAmount is set to 2000 ETH which is a plausible amount for a protocol of this kind. Restricting derivates to Reth allowed me to test the code swap when Rocket Pool is not able to be used directly so UniswapV3 is used to swap WETH for RETH.

The issue is caused by an overflow in poolPrice() L241 which causes the staking operation to revert. Following the execution path;

1. SafEth::stake(1596045421752747910003) on L63 is called with a value such as 1596045421752747910003 wei (~1596 ETH).
2. SafEth deposits the amount of ETH into the Reth derivative contract calling deposit{value: 1596045421752747910003}() on L91.
3. The ETH is transferred, deposited as WETH and then swapped to RETH using the UniswapV3 pool in Reth.sol. In this example is returns 1483574263903969556156 wei or approximately 1483 (staked) ETH.
4. ethPerDerivative(1483574263903969556156) on L92 is called to get the value of StakedETH (RETH in this case). This passes through to the Reth.sol contract ethPerDerivative() function on L211.
5. As the deposit is more than can be made directly to RocketPool poolPrice() L228 is used to get the ethPerDerivative value via UniswapV3.
6. This reads the Uniswap pool slot0 which returns a uint160 price that is converted into a non-square root price L241 and overflows causing a revert.
   1. e.g. return (sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18)) >> (96 * 2);

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "forge-std/Vm.sol";
import "forge-std/console.sol";
import "../contracts/SafEth/SafEth.sol";
import "../contracts/SafEth/derivatives/Reth.sol";
import "../contracts/SafEth/derivatives/SfrxEth.sol";
import "../contracts/SafEth/derivatives/WstEth.sol";

import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import "../contracts/interfaces/uniswap/IUniswapV3Factory.sol";
import "../contracts/interfaces/uniswap/IUniswapV3Pool.sol";

contract SafEthTest is Test {
    Reth rethImp;
    SfrxEth sfrxImp;
    WstEth wstImp;
    SafEth safethImp;
    ERC1967Proxy safeEthproxy;
    ERC1967Proxy rethProxy;
    ERC1967Proxy wstProxy;
    ERC1967Proxy sfrxProxy;

    address public constant ROCKET_STORAGE_ADDRESS =
        0x1d8f8f00cfa6758d7bE78336684788Fb0ee0Fa46;
    address public constant W_ETH_ADDRESS =
        0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
    address public constant UNISWAP_ROUTER =
        0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45;
    address public constant UNI_V3_FACTORY =
        0x1F98431c8aD98523631AE4a59f267346ea31F984;

    function setUp() public {
        safethImp = new SafEth();
        safeEthproxy = new ERC1967Proxy(address(safethImp), 
            abi.encodeWithSelector(SafEth.initialize.selector,"Asymmetry Finance ETH","safETH"));

        rethImp = new Reth();
        rethProxy = new ERC1967Proxy(address(rethImp), 
            abi.encodeWithSelector(Reth.initialize.selector, address(safeEthproxy)));

        SafEth(payable(address(safeEthproxy))).addDerivative(address(rethProxy), 1 ether);

        wstImp = new WstEth();
        wstProxy = new ERC1967Proxy(address(wstImp), 
            abi.encodeWithSelector(WstEth.initialize.selector, address(safeEthproxy)));

        //SafEth(payable(address(safeEthproxy))).addDerivative(address(wstProxy), 1 ether);

        sfrxImp = new SfrxEth();
        sfrxProxy = new ERC1967Proxy(address(sfrxImp), 
            abi.encodeWithSelector(SfrxEth.initialize.selector, address(safeEthproxy))); 
        
        //SafEth(payable(address(safeEthproxy))).addDerivative(address(sfrxProxy), 1 ether);
    }

    function testStaking(uint256 amount_in) public {
        SafEth s = SafEth(payable(address(safeEthproxy)));
        s.setMaxAmount(2000 ether);

        vm.assume(amount_in >=  s.minAmount());
        vm.assume(amount_in <= s.maxAmount());

        vm.deal(address(1337), amount_in);

        vm.startPrank(address(1337));
        s.stake{value: amount_in}();
        vm.stopPrank();

        assert(s.balanceOf(address(1337)) >  0);

        vm.startPrank(address(1337));
        s.unstake(s.balanceOf(address(1337)));
        vm.stopPrank(); 
    }

An example remappings.txt is;

@chainlink/=node_modules/@chainlink/
@ensdomains/=node_modules/@ensdomains/
@eth-optimism/=node_modules/@eth-optimism/
@openzeppelin/=node_modules/@openzeppelin/
ds-test/=lib/forge-std/lib/ds-test/src/
eth-gas-reporter/=node_modules/eth-gas-reporter/
forge-std/=lib/forge-std/src/
hardhat-deploy/=node_modules/hardhat-deploy/
hardhat/=node_modules/hardhat/
v3-core/=lib/v3-core/

To run the test above you can execute the following steps in the project directory;

1. forge init --no-commit --force
2. Delete default forge contract files (test, src, and script).
3. Adjust the remappings.txt to look like the one above.
4. Copy the test above into the test/ directory and name the file SafEthTest.t.sol.
5. Then run forge test -vvvvv --fork-url=$FORK_URL --match-test=testStaking
6. This will revert because of the overflow in poolPrice().

Recommended mitigation steps

The UniswapV3 TWAP should be used instead of reading directly from pool.slot0 . This is beneficial from a price perspective and will also help mitigate some price manipulation where slot0 is manipulated via Flashloans.