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

14 stars 12 forks source link

limited liquidity in rETH/ETH pool can cause rebalancing calls to fail #107

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-L204 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L152

Vulnerability details

Impact

rETH only allows a limited amount of ETH to be deposited directly. The Uniswap pool used to swap from ETH to rETH has very low liquidity. When SafETH is rebalanced, all of the ETH it holds is withdrawn and re-deposited to each protocol. Depending on the size of SafETH's ETH holdings, the deposit can exceed Rocket Pool's deposit limit and the Uniswap pool's liquidity. That will cause the rebalancing tx to revert. That would limit the protocol's exposure to RocketPool. While that may sound like a far-fetched issue because of the deposit size, a quick look at DefiLlama will show that it is quite reasonable:

Since SafETH allows users to diversify their ETH into two protocol's you would expect it to take liquidity away from both Lido and Rocket Pool. If SafETH holds 10k ETH (0.15% of Lido & Rocket Pool holdings) and distributes it 50/50, it will already reach Rocket Pool's deposit limit (given the deposit pool is empty).

At that point, it won't be able to rebalance the weights such that Rocket Pool would receive more than 5000 ETH because the transaction would revert.

Proof of Concept

When SafETH is rebalanced, all the ETH it holds is withdrawn and then redeposited

    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();
    }

The Reth adapter implements two ways to deposit funds:

    function deposit() external payable onlyOwner returns (uint256) {
        // Per RocketPool Docs query addresses each time it is used
        address rocketDepositPoolAddress = RocketStorageInterface(
            ROCKET_STORAGE_ADDRESS
        ).getAddress(
                keccak256(
                    abi.encodePacked("contract.address", "rocketDepositPool")
                )
            );

        RocketDepositPoolInterface rocketDepositPool = RocketDepositPoolInterface(
                rocketDepositPoolAddress
            );

        if (!poolCanDeposit(msg.value)) {
            uint rethPerEth = (10 ** 36) / poolPrice();

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

            IWETH(W_ETH_ADDRESS).deposit{value: msg.value}();
            uint256 amountSwapped = swapExactInputSingleHop(
                W_ETH_ADDRESS,
                rethAddress(),
                500,
                msg.value,
                minOut
            );

            return amountSwapped;
        } else {
            address rocketTokenRETHAddress = RocketStorageInterface(
                ROCKET_STORAGE_ADDRESS
            ).getAddress(
                    keccak256(
                        abi.encodePacked("contract.address", "rocketTokenRETH")
                    )
                );
            RocketTokenRETHInterface rocketTokenRETH = RocketTokenRETHInterface(
                rocketTokenRETHAddress
            );
            uint256 rethBalance1 = rocketTokenRETH.balanceOf(address(this));
            rocketDepositPool.deposit{value: msg.value}();
            uint256 rethBalance2 = rocketTokenRETH.balanceOf(address(this));
            require(rethBalance2 > rethBalance1, "No rETH was minted");
            uint256 rethMinted = rethBalance2 - rethBalance1;
            return (rethMinted);
        }
    }

poolCanDeposit() checks whether the deposit amount is within bounds: deposit pool balance + amount less than 5000e18 and more than 0.01e18:

    function poolCanDeposit(uint256 _amount) private view returns (bool) {
        address rocketDepositPoolAddress = RocketStorageInterface(
            ROCKET_STORAGE_ADDRESS
        ).getAddress(
                keccak256(
                    abi.encodePacked("contract.address", "rocketDepositPool")
                )
            );
        RocketDepositPoolInterface rocketDepositPool = RocketDepositPoolInterface(
                rocketDepositPoolAddress
            );

        address rocketProtocolSettingsAddress = RocketStorageInterface(
            ROCKET_STORAGE_ADDRESS
        ).getAddress(
                keccak256(
                    abi.encodePacked(
                        "contract.address",
                        "rocketDAOProtocolSettingsDeposit"
                    )
                )
            );
        RocketDAOProtocolSettingsDepositInterface rocketDAOProtocolSettingsDeposit = RocketDAOProtocolSettingsDepositInterface(
                rocketProtocolSettingsAddress
            );

        return
            rocketDepositPool.getBalance() + _amount <=
            rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() &&
            _amount >= rocketDAOProtocolSettingsDeposit.getMinimumDeposit();
    }

Currently, the pool is already full (input is "rocketDepositPool"). So you won't be able to use that. Instead, the deposit has to be executed through Uniswap. But, the Uniswap pool doesn't have an infinite amount of liquidity. It's actually quite limited. Currently, it only holds 1.62k rETH and 1.28k ETH: https://info.uniswap.org/#/pools/0xa4e0faa58465a2d369aa21b3e42d43374c6f9613

Meaning, if you try to deposit more ETH than there is rETH in the pool the Uniswap path will also fail because of the slippage protection. The swap won't pass the minOut check:

        if (!poolCanDeposit(msg.value)) {
            uint rethPerEth = (10 ** 36) / poolPrice();

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

            IWETH(W_ETH_ADDRESS).deposit{value: msg.value}();
            uint256 amountSwapped = swapExactInputSingleHop(
                W_ETH_ADDRESS,
                rethAddress(),
                500,
                msg.value,
                minOut
            );

            return amountSwapped;

To sum up, you're not able to deposit ETH that's worth more than ~1.5k rETH in one go. It strictly limits the protocol's exposure to Rocket Pool (2nd largest liquid staking protocol).

The withdrawal is also an issue. The Rocket Pool contracts don't have unlimited ETH to cover withdrawals. At most, you can only withdraw rETH worth RocketTokenRETH.getTotalCollateral(): https://etherscan.io/address/0xae78736cd615f374d3085123a210448e74fc6393#code#F6#L139 At the time of writing that's 5537 ETH: https://etherscan.io/address/0xae78736cd615f374d3085123a210448e74fc6393#readContract#F8

So if the SafETH contract holds rETH worth more than 5537 ETH, the rebalancing function will revert.

Tools Used

none

Recommended Mitigation Steps

Instead of depositing the whole amount at once, it should be done in multiple steps. The easiest solution would be to use a timelock that has the ability to freely move funds between the different protocols. That allows more granular rebalancing of funds, and transparency and time for users to react to changes in exposure to different protocols.

Another temporary solution is to use the Balancer pool. It has deeper liquidity: https://app.balancer.fi/#/ethereum/pool/0x1e19cf2d73a72ef1332c882f20534b6519be0276000200000000000000000112

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #673

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory