Asymmetry contest - 0Kage's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L142 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L108

Vulnerability details

Impact

rebalanceToWeights function is expected to be called by Owner to force-exit all positions across all derivatives & re-enter positions based on new weights. Implicit assumption in the function is that all derivative positions can be exited.

While this may be true for stEth and fraxEth by increasing the maxSlippage to appropriate level, it might not be possible to exit Rocket Pool position that does not use an AMM. In case of Rocket Pool, a successful Eth transfer is subject to the availability of excess ETH in rocket-pool vaults. This ETH is taken from queued pools that are awaiting deployment.

Since rebalanceToWeights closes the entire position in one shot, it is likely that the excess ETH in rocket pool vaults is not enough to execute a successful withdrawal request. And since rebalanceToWeights is only successful if withdrawals across all derivatives is successful, protocol owners will be helpless in de-peg scenarios where they want to quickly exit the position & save from further losses

Eg. if stEth starts to depeg, exit from stEth is prevented by DDOS of rocket pool withdrawals.

Proof of Concept

Check Line 140 in etherscan code for RocketTokenRETH contract.

    // Burn rETH for ETH
    function burn(uint256 _rethAmount) override external {
        // Check rETH amount
        require(_rethAmount > 0, "Invalid token burn amount");
        require(balanceOf(msg.sender) >= _rethAmount, "Insufficient rETH balance");
        // Get ETH amount
        uint256 ethAmount = getEthValue(_rethAmount);
        // Get & check ETH balance
        uint256 ethBalance = getTotalCollateral();
        require(ethBalance >= ethAmount, "Insufficient ETH balance for exchange"); //@audit txn reverts if ethBalance is less than ethAmount
        // Update balance & supply
        _burn(msg.sender, _rethAmount);
        // Withdraw ETH from deposit pool if required
        withdrawDepositCollateral(ethAmount);
        // Transfer ETH to sender
        msg.sender.transfer(ethAmount);
        // Emit tokens burned event
        emit TokensBurned(msg.sender, _rethAmount, ethAmount, block.timestamp);
    }

ethBalance is calculated using getTotalCollateral function that calls the getExcessBalance on rocketDepositPool in Line 99

    // Get the total amount of collateral available
    // Includes rETH contract balance & excess deposit pool balance
    function getTotalCollateral() override public view returns (uint256) {
        RocketDepositPoolInterface rocketDepositPool = RocketDepositPoolInterface(getContractAddress("rocketDepositPool"));
        return rocketDepositPool.getExcessBalance().add(address(this).balance);
    }

On checking RocketDepositPool contract on etherscan, note that on Line 88, effective capacity is calculated using pool queue that is then used to calculate excess balance available for transfer. If excess capacity is not available, there isn't enough ETH to honour a large withdrawal request

    // Excess deposit pool balance (in excess of minipool queue capacity)
    function getExcessBalance() override public view returns (uint256) {
        // Get minipool queue capacity
        RocketMinipoolQueueInterface rocketMinipoolQueue = RocketMinipoolQueueInterface(getContractAddress("rocketMinipoolQueue"));
        uint256 minipoolCapacity = rocketMinipoolQueue.getEffectiveCapacity();
        // Calculate and return
        uint256 balance = getBalance();
        if (minipoolCapacity >= balance) { return 0; }
        else { return balance.sub(minipoolCapacity); }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using a try-catch that disconnects exits across different derivatives. Even if one derivative has a problem with exiting, others can exit quickly.

Lines of code

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

Vulnerability details

Impact

poolPrice is used to calculate the price of reth in terms of eth. To do this, the function is calling slot0 on UniswapV3 Pool. slot0 captures the most recent token balance and hence can be easily manipulated.

As per Uniswap V3 docs, slot0 stores the current tick of the pool. Since it is only a single tick, price can be easily manipulated by creating instantaneous token imbalances in the pool.

Proof of Concept

deposit function calculates the minOut parameter (minimum rEth while swapping for WETH) using the poolPrice. If poolPrice is momentarily inflated by manipulating latest tick of Uniswap pool, minOut can be significantly low. This can potentially cause a large slippage while swapping WETH to reth causing a loss to depositors.

 function deposit() external payable onlyOwner returns (uint256) {
   ...
        if (!poolCanDeposit(msg.value)) {
            uint rethPerEth = (10 ** 36) / poolPrice(); //@audit -> price uses slot0 - can be manipulated

            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;
        }
   ...
}

Tools Used

Manual Review

Recommended Mitigation Steps

Use TWAP price instead of slot0 price in poolPrice function

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L142

Vulnerability details

Impact

In a de-peg scenario, there will be a general flight to safety (ETH) from users across the board. All pools will have a uni-directional sell-pressure where users prefer to exchange derivative token for WETH.

There are 3 sources of losses in this scenario

• Protocol currently doesn't localize exit & force-exits all positions. So even non de-pegged assets are forced to exit causing further sell-side pressure that can further widen slippages

• Protocol immediately tries to re-enter the position based on new weights. Since de-peg in one asset can trigger de-peg in another (eg. USDT de-pegged immediately after UST collapse), immediately entering into another position after exiting one might cause more. A better approach would be to simply exit stressed positions & waiting out for the market/gas prices to settle down. Separating exit & re-entry functions can save depositors from high execution costs.

• Protocol is inefficiently exiting & re-entering the positions. Instead of taking marginal positions to rebalance, current implementation first fully exits & then re-enters back based on new weights (see POC below). Since any slippage losses are borne by depositors, a better implementation can save losses to users

Proof of Concept

• Assume positions are split in following ratio by value: 10% frax-Eth, 70% stEth and 20% rEth
• Now frax-Eth starts to de-peg, forcing protocol to exit frax-Eth and rebalance to say, 80% stEth and 20% rEth
• Current rebalancing first exits 70% stEth, 20% rEth and then re-enters 80% stEth and 20% rEth
• A marginal re-balancing would have only needed protocol to exit 10% frax-Eth and divert that 10% to stEth

By executing huge, unnecessary txns, protocol is exposing depositors to high slippage costs on the entire pool.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider following improvements to rebalanceToWeights:

• Separate exits & entries. Split the functionality to exit and re-enter. In stressed times or fast evolving de-peg scenarios, protocol owners should first ensure an orderly exit. And then wait for markets to settle down before re-entering new positions

• Localize exits, ie. if derivative A is de-pegging, first try to exit that derivative position before incurring exit costs on derivative B and C

• Implement a marginal re-balancing - for protocol's whose weights have increased, avoid exit and re-entry. Instead just increment/decrement based on marginal changes in net positions