Asymmetry contest - gjaldon's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L71-L99 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L211-L216 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L146-L150 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L170-L186

Vulnerability details

Impact

SafEth uses a different Price Oracle depending on whether Reth.poolCanDeposit(uint256) returns true or false. If true, it uses the RocketPool price oracle. Otherwise, it uses the UniswapV3 pool's price oracle. Since these are different oracles, they will return different prices. The UniswapV3 pool's price oracle is more greatly affected by demand even though it's a TWAP. A sustained sell-off in the pool will lead to a drop in price or a sustained high demand will lead to an increase in price. It is this disparity in price between the 2 oracles that can lead to users of the protocol losing some of their ETH.

Proof of Concept

Given the following:

1. rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() returns 5e21
2. rocketDepositPool.getBalance() returns 49e20
3. UniswapV3 Price Oracle RETH price in ETH = 1.29
4. Rocket Price Oracle RETH price in ETH = 1.08 - 10% less than price in UniswapV3
5. RETH is the only Derivative in SafEth with weight
6. RETH Derivative contract has a balance of 100 ETH

A user stakes 200 ETH by calling SafEth.stake().

        for (uint i = 0; i < derivativeCount; i++)
            underlyingValue +=
                (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
                    derivatives[i].balance()) /
                10 ** 18;

Since derivatives[i].balance() returns 100 ETH, derivatives[i].ethPerDerivative(100 ether) will return 1.08 which is the RocketPool oracle price. This is because Reth.poolCanDeposit(100 ether) will return true:

        return
            rocketDepositPool.getBalance() + _amount <= // this will be 5e21 which is equal to maximumDepositPoolSize
            rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() && // this will be 5e21
            _amount >= rocketDAOProtocolSettingsDeposit.getMinimumDeposit(); // this will be true

However, when computing for the amount of ETH staked, the UniswapV3 price oracle is used which returns 1.29 in price:

            uint256 depositAmount = derivative.deposit{value: ethAmount}();
            uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
                depositAmount
            ) * depositAmount) / 10 ** 18;
            totalStakeValueEth += derivativeReceivedEthValue;

depositAmount above is ~155 RETH (200 ETH / 1.29 ignoring swap fees and slippage). totalStakeValueEth = ~155 ETH * 1.29 = ~200 ETH.

To get the amount of SafEth tokens minted, we look at the following code:

preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;
//snip...
uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;

underlyingValue is computed based on price from RocketPool oracle and preDepositPrice is underlyingValue divided by totalSupply:

underlyingValue = 1.08 * (100 ETH / 1.29) = ~83.7 ETH
preDepositPrice = `83.7 ETH / (100 ETH / 1.29 (price of RETH)) 
                = ~1.08 ETH

We can now get the mint amount by substituting the values:

mintAmount = 200 ETH / ~1.08 ETH
           = ~185.19 SafEth tokens

The main takeaway here is that preDepositPrice is 1.08 which is 10% lower than 1.29 from the UniswapV3 oracle which has led to more SafEth tokens being minted than RETH tokens minted. Recall that the user got ~185 SafEth tokens even though Reth tokens minted was only ~155.

The updated totalSupply for the tokens are now:

1. SafEth totalSupply = ~185 SafEth + ~77.5 SafEth = ~262.5 SafEth
2. Reth balance of RethDerivative contract = ~155 Reth + ~77.5 Reth = ~232.5 Reth

When the user calls SafEth.unstake(185 Eth), he gets the following:

uint256 derivativeAmount = (derivatives[i].balance() *
                _safEthAmount) / safEthTotalSupply

232.5 Reth * 185 SafEth / 262.5 SafEth = ~163.86 Reth tokens

The user gets more Reth tokens, ~163.86 Reth tokens, even though his staking action only minted ~155 Reth tokens. These are tokens that belong to other depositors. When withdraw is called with ~163.86 tokens, the user ends up with 163.86 * 1.29 = 211.37 Eth.

That's ~11 ETH of profit at the expense of other users.

Tools Used

Manual Review, VSCode, Hardhat

Recommended Mitigation Steps

All calls of Reth.ethPerDerivative(uint256) in SafEth.stake() and the call to poolCanDeposit(uint256) in Reth.deposit() should use the same argument so that all computations will use the same price from the same oracle. Below are the affected lines:

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

Reth.sol#L170 uses the correct Ether value. The ethPerDerivative() calls in stake() should use ethAmount and not depositAmount or derivatives[i].balance().