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

14 stars 12 forks source link

Reth.sol: Hardcoded Uniswap V3 pool fee reduces future-proofing and exposes the protocol to MEV #123

Open code423n4 opened 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#L177-L183 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L83-L102

Vulnerability details

Impact

A user staking with Asymmetry loses value immediately when the protocol fetches too little rETH tokens on the rETH leg, assuming some external conditions.

Proof of Concept

Reth.sol's deposit function aims to convert ETH to rETH tokens. It normally uses RocketPool's staking mechanism (RocketDepositPool), but may instead fall back to a conversion via Uniswap V3 due to a temporary inability of the deposit pool to accept new deposits or due to the amount being less than the minimum threshold.

In the fallback mechanism Reth.sol#L177-L183 swaps WETH to rETH via the 0.05%-fee Uniswap V3 WETH/rETH pool, where the fee is hard-coded.

However, there is no guarantee that the 0.5%-fee pool will remain the most liquid, or at all liquid, in the future. Liquidity may migrate to another fee level (say, 0.1%) or flee altogether. This has happened with other pairs before due to external factors, such as the endorsement of another pool by the protocol's team; a new liquidity mining incentive for another fee level; or a popular integration such as a yield strategy.

A low-liquidity Uniswap V3 pool opens the protocol to a few negative externalities:

First, bad / out-of-sync prices (the slippage protection mechanism does not protect against this as it takes the price quote from the same pool).

Second, toxic MEV activity: sandwich attacks, JIT liquidity attacks, oracle manipulation. The prerequisite for these attacks is that the swap size is large relative to the available liquidity in the pool. The attacks essentially reduce the amount of tokens the victim receives.

Tools Used

hardhat, vscode, brain

Recommended Mitigation Steps

One possibility is to abstract the fee level configuration to a variable, poolFee, set to 500 in the constructor. A new setter function, setPoolFee allows the owner or governor to set the pool fee.

Another suggestion is to add minimum liquidity requirements for the selected pool, prior to swapping.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #916

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

Picodes marked the issue as grade-b