Asymmetry contest - roelio's results

Lines of code

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

Vulnerability details

Impact

When ETH cannot be directly deposited into the Rocket pool according to https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L120 the price will be calculated from the current state of the uniswap pool https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L240.

If the balance of the RETH derivative is high enough to trigger the use of the uniswap oracle in https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L73. The preDepositPrice can be manipulated by skewing the uniswap pool resulting in a higher mintAmount in https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L98.

Proof of Concept

pre-conditions:

Only RETH as derivative and balance is high enough to make ethPerDerivative use the Uniswap pool in https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L73

Price of 1.1 ETH per RETH in the Uniswap pool and Rocket pool.

Underlying value of stake pool is 1100 ETH and total supply is 1100.

RETH derivative contains 1000 RETH.

Attack:

Attacker flashloans bunch of RETH and swaps for ETH. Making the price in the Uniswap pool 1 ETH per RETH.

Because the price is determined via uniswap the underlyingvalue of the pool calculated in https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L73 will be 1000. So the predeposit price will be ~0.909 ETH

The totalStakeValueEth in https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L92 will be 100 ETH because the price used their is coming from the RocketPool (not uniswap).

The mintAmount will be 110 ETH

Now the uniswap is skewed to its original ratio.

Underlying value of stake pool is 1200 ETH and total supply is 1210.

Now the attacker can withdraw ~109.09 ETH base on the 110 shares.

Tools Used

Notepad

Calculator

Recommended Mitigation Steps

Use a TWAP from the Uniswap pool or possible chainlink if available.

Another problem is when the price calculated for the Rocket derivative in https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L73 could be different w.r.t. https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L92. In the first calculation it should use the price based on the deposit amount and not the balance of the pool.

Overall code is simple and self-explanatory. The codebase is small and to the point. Modern solidity is applied.

Low severity

Wrong price of the lido derivative

• Problem: https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L87 returns the amount of stETH for one wstETH. And it should return the amount of ETH for one wstETH.

• Solution: Use the curve or chainlink oracle to determine the amount of ETH per wstETH

Adjusting weights of non-existing derivatives

• Problem: It is possible to set the weight of a derivative that doesnt exist https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L169. Though it doesnt cause issues its probably good to revert as feedback.

• Solution: require the derivative to exist

Slippage should be limited

• Problem: Both https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L52 and https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L49 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L59 should probably be restricted to not allow extreme slippage which could be used to do a malicious arbitrage.

• Solution: Have an upper bound to the slippage. A one for all solution could be done in https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L206

Owner can hijack funds by adding derivative and adjusting weights.

• Problem: function https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L182 allows the owner to add a malicious derivative and could then change weights to move all eth to this contract and steal the funds.

• Solution: In this case the owner should ideally be behind a timelock, but maybe this is not okay for pausing the staking so potentially add an additional role for immediate actions and put the rest behind timelock.

Overflow in localTotalWeight calculation

• Problem: In the scenario where the weights add up to be more than the uint256 can hold the transaction will revert (https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L192, https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L172)

• Solution: Use a smaller uint8 or similar for storing the weights.