Asymmetry contest - carrotsmuggler's results

Lines of code

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

Vulnerability details

Impact

The contract is susceptible to the common price manipulation scenario, where a single wei is minted and the ratio is manipulated. The contract has a minimum deposit measure in place, however that can be circumvented by a withdraw following the deposit, keeping only a single wei of safEth alive, so that totalSupply != 0

Proof of Concept

The attack can be carried out in the following steps:

1. First user deposits 0.5 eth (minimum amount). They then withdraws all except 1 wei of safEth. This costs them dust amounts of eth, and the contract holds ~1 wei of reth, wsteth and sfrxeth, and has minted 1 wei of safEth.
2. Attacker sends 1 reth, 1 wsteth and 1 sfrxeth in the relevant contracts.
3. Victim tries to stake 2 eth.
   • Underlying value = 3e18 + 3 wei (from attackers "donation" in step 1 + remaining dust)
   • totalSupply = 1 (after deposit and withdraw of attacker)
   • predepositprice = 1e18 * 3e18 / 1 = 3e36 + 3e18 (huge decimals! second term is the dust value)
   • totalStakeValue = 1e18 (victims 1 eth, split into 3, assumed to be equal to 1 eth)
   • mintAmount = 1e18 * 1e18 / (3e36 + 3e18) = 1 wei
4. Victim only minted 1 wei, instantly loses access to half their funds

Tools Used

Manual review

Recommended Mitigation Steps

Burn the initial ~1000 wei tokens minted, such that contract never reached a state where the ratio can be manipulated.

Lines of code

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

Vulnerability details

Impact

The contract does not support the removal of any derivatives. Instead, it implements a weight mechanism, and assumes that a weight = 0 is equivalent to that derivative not existing. This is true for stake and rebalanceToWeights, but is violated during unstake. Thus derivatives tracked in the contract are never ignored. This can lead to the breaking of withdrawals even if the protocol had no exposure to the derivative, as can be shown in the following POC.

Proof of Concept

Let's assume rocketpool is under investigation by the SEC. Anticipating a negative outcome, the protocol sets the weight of reth to 0, and rebalances the pool. All reth is sold for sfrxeth and wsteth, essentially reducing the protocols exposure to rocketpool to 0.

SEC shuts down rocketpool. Due to market instability, rocketpool withdrawals become insolvent. Protocol isn't affected, since it didn't hold any reth. Unstake operations work perfectly, since the following code block prevents any interaction with the reth contracts.

uint256 derivativeAmount = (derivatives[i].balance() *
                _safEthAmount) / safEthTotalSupply;
            if (derivativeAmount == 0) continue; 

Attacker sends the contract 1 reth. Now, the unstake operation ceases to work, since reth contract is insolvent, and thus withdraw function keeps reverting. Even though the protocol had no exposure, it is still bricked since it tries to withdraw from the reth contract unnecessarily.

Tools Used

Manual Review

Recommended Mitigation Steps

Add functionality to remove derivatives completely.

Lines of code

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

Vulnerability details

Impact

The price of the derivatives are returned by the ethPerDerivative functions in the corresponding derivative contracts. The prices are calculated in different ways:

1. For sfrxeth: price = amount of frxEth per sfrxeth * price of frxEth on Curve
2. For reth: price = price of reth on uniswapV3
3. For wstEth: price = amount of stEth per wstEth

This is problematic, since for sfrxeth and reth, the prices are in eth, while for wstEth the price is actually in stEth, not eth. This snippet shows the price oracle used.

    function ethPerDerivative(uint256 _amount) public view returns (uint256) {
        return IWStETH(WST_ETH).getStETHByWstETH(10 ** 18);
    }

Currently, 1 stEth != 1 eth. I would argue that even after the Shanghai upgrade, stEth and Eth will not reach parity, since stEth holders take on the additional risk of solvency of LIDO, node operators shutting down nodes to provide exit liquidity, smart contract risks, etc. Thus stEth will ALWAYS be cheaper than eth, since these risks are calculated into the price.

Thus in this case, since the amount of stEth is used, which is worth less than the same amount of eth, the contract overvalues wstEth. Also, the withdrawal mechanism for wstEth uses a curve pool, and thus is always subject to current market prices of stEth, and should use the same when calculating its net worth.

Proof of Concept

There are routinely 1% difference in prices between stEth and eth. Also events like steth depeg clearly shows that a lot of trust is placed on LIDO which might not amount to any tangible value.

A 1% difference essentially means that wstEth is valued 1% higher than the other derivatives. If the difference in prices grow, which can be the case due to liquidity crunches which are quite realistic, wstEth will be valued even higher than the other derivatives.

Liquidity crunches are common since node operators are required to shut down their nodes to free up liquid eth, which takes time.

Tools Used

Manual Review

Recommended Mitigation Steps

Use a chainlink price feed to get the price of stEth. Return price = IWStETH(WST_ETH).getStETHByWstETH(10 ** 18) * chainlink_price / 1e18;

Lines of code

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

Vulnerability details

Impact

Rocketpool withdrawals are not always solvent. In rocketpool, users stake 16 eth and node operators stake the remaining 16 eth. A minimum quantity of eth is left in the contract for withdrawals, but the majority of eth are tied up in staking. So users cannot redeem all reth for eth, since most are controlled by node operators.

Even after the shanghai upgrade, reth will still remain insolvent. This is because when the liquid eth in the contract dries up, the contract has to wait for a node operator to shut down their node and free up more eth. This is profitable for node operators since an insolvent reth contract drops the reth price, allowing node operators to profit from the price drop. Stakers are still expected to wait until enough eth is freed up before burning reth. Users who don't want to wait are thus encouraged to swap their reth for eth in LPs, reducing load from the withdrawal mechanism.

In this protocol, however, the only method for withdrawals of reth is the burn mechanism, which can be insolvent. Thus all users of the protocol have to wait until reth contract has liquidity, in order to reduce their positions. Other derivatives also suffer from the same issue, but have alternatives implemented such as curve or uniswap pools. This is however not implemented for the reth contract.

If the protocol decides to forgo reth and set its weight to 0, it willstill deal with this issue. The unstake function does not care about weights, and tries to withdraw all derivatives in the contracts. If an attacker sends trace amounts of reth to the derivative contract, the protocol will try to burn it and revert, bricking the unstake functionality even when weights are set to 0.

Proof of Concept

Withdrawals will not work when rocketpool contract doesn't have enough liquid eth. Since this is the bottleneck, the entire functionality of the protocol depends on the liquidity in the reth deposit contract.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement an alternative option to swap reth in an LP like uniswap in case enough liquidity is not available. This is already implemented for the stake option, and should be present for the unstake option as well.

Lines of code

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

Vulnerability details

Impact

The poolPrice function in Reth.sol contract uses the value sqrtPriceX96 stored in slot0 of the uniswap contract. This value however is the current tick, and is thus the spot price of the pool. Thus this value can be easily manipulated by taking flashloans and manipulating the composition of the pool.

In function stake, the ethPerDerivative value is used to calculate the underlyingValue before deposit. The sums of the underLyingValue make up the preDepositPrice. The ethPerDerivative function in Reth uses two logics to calculate the price ratio as seen from this code block

function ethPerDerivative(uint256 _amount) public view returns (uint256) {
    if (poolCanDeposit(_amount))
        return RocketTokenRETHInterface(rethAddress()).getEthValue(10 ** 18);
    else return (poolPrice() * 10 ** 18) / (10 ** 18);
}

The issue is that the stake function can be made to use these two different values at two different points in the same function. This is shown in the following statements.

            underlyingValue +=
                (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
                    derivatives[i].balance()) /
                10 ** 18;

            uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
                depositAmount
            ) * depositAmount) / 10 ** 18;

Which logic is used depends on the amount passed to ethPerDerivative. As we can see here, in the first case, the ENTIRE balance of the contract is passed. If this value is large, and hits the rocketpool cap, then it will force the contract to use the uniswap spot price, which can be manipulated. The pool can be manipulated to return a smaller price. The second block shows only the user passed amount is passed, which is smaller value and likely to return the reth contract's tracked price. The amount of safEth minted comes from the following statement

uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;

PreDepositPrice uses the manipulated uniswap pool price and sends a small price, while totalStakedValueEth uses the normal price. Thus more safEth tokens get minted, profiting the attacker.

Once the balance held in the contract crosses a limit, it is highly susceptible to price manipulation attacks.

Proof of Concept

Attack needs following conditions: Reth.sol derivative contract must have sufficient reth in it so that poolCanDeposit call returns false when its entire balance is queried.

Steps:

1. Take a flashloan and manipulate uniswap reth-weth pool to drop its reth price in eth.
2. Call stake with some amount
3. Rebalance uniswap pool.

If the edge gained from manipulated price is greater than the uniswap fee + flasloan fee, the attacker profits.

Tools Used

Manual Review

Recommended Mitigation Steps

1. Use chainlink feed for reth price, instead of uniswap pool price
2. When calculating underlyingValue, pass 0 to the ethPerDerivative function. This is because calculating underlying step does not need to check if the pool is open for deposits. That check is done later in the actual deposit anyways.

Lines of code

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

Vulnerability details

Impact

The price oracle for sfrxEth is the CURVE pool for frx-weth. This pool uses the spot price, which can be manipulated to provide an edge for depositors when the derivative holdings are unbalanced.

Artificially depressing this price will depress the underlyingValue of the contract, which is used to determine how many safEth tokens get minted. If the balances in the contract are not perfectly correlated to the weights, then this manipulation can be used to pull off profitable attacks as demonstrated below.

Proof of Concept

Since the spot price is used, we can assume the ethPerDerivative can be manipulated, and lets assume as an extreme case, where the price oracle is manipulated to 0.5.

Assumptions

Let's assume the weights are equal, and thus equal amounts are dropped into all three derivative pools. User is depositing 3 eth, which gets split into 1 reth, 1 sfrxeth and 1 wsteth.

Let's also assume the actual contract has holdings which are different from the weights, i.e. the holdings in all three protocols are not equal. This can be due to external deposits or a recent change in weights. We assume the contract holds 1000 wsteth, 1000 reth and 4000 sfrxeth, as an extreme case for easy maths.

We explore two cases, one with manipulated ethPerDerivative of 0.5, and one with normal prices of 1.0

Assume totalSupply = 1e18

The relevant formulas for calculating the values are given below and taken from the contract

underlyingValue +=
                (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
                    derivatives[i].balance()) /
                10 ** 18;

preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
                depositAmount
            ) * depositAmount) / 10 ** 18;

uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;

Case 1 - ethPerDerivative = 1e18 for all

underlying value total = 4000 * 1 (sfrxeth) + 1000 * 1 (reth) + 1000 * 1 (wsteth) = 6000e18 preDepositPrice = 1e18 * 6000e18 * 1e18 = 6000e18 derivativeReceivedEthValue for each = 1e18 * 1e18 / 1e18 = 1e18 totalStakeValueEth = 3e18 mintAmount = 3e18 * 1e18 / 6000e18 = 0.0005e18 safEth tokens minted

Case 2 - ethPerDerivative = 0.5e18 for sfrxeth, 1e18 for others

underlying value total = 4000 * 0.5 (sfrxeth) + 1000 * 1 (reth) + 1000 * 1 (wsteth) = 4000e18 preDepositPrice = 1e18 * 4000e18 * 1e18 = 4000e18 derivativeReceivedEthValue for reth and wsteth = 1e18 * 1e18 / 1e18 = 1e18 derivativeReceivedEthValue for sfrxeth = 0.5e18 * 1e18 / 1e18 = 0.5e18 totalStakeValueEth = 2.5e18 mintAmount = 2.5e18 * 1e18 / 4000e18 = 0.625e18 safEth tokens minted

Thus case 2 (with manipulation) mints more tokens than case 1. So if tokens in the contract are not balanced, price oracles can be manipulated to gain extra edge. Practically speaking the differences and edge will be much smaller, but here a case with 1250x profit is shown as an extreme case to highlight the issue.

This issues is also present for reth, which also uses spot price, but there is another big issue for that contract which is discussed in a separate report.

Tools Used

Manual Review

Recommended Mitigation Steps

Use chainlink feed to get price of sfrxEth in terms of ETH.