code-423n4 / 2023-05-asymmetry-mitigation-findings

2 stars 2 forks source link

Mitigation Confirmed for Mitigation of H-08: Issue mitigated with error #35

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Mitigated issue

H-08: Staking, unstaking and rebalanceToWeight can be sandwiched (Mainly rETH deposit).

The issue was a MEV sandwich attack on withdraw() (called by unstake()). In Reth simply because slippage protection was missing, and in SfrxEth because the slippage was evaluated within the transaction rather than when it was sent. The latter case is exploitable if the price used to evaluate the slippage can be manipulated. In SfrxEth FrxEthEthPool.price_oracle() was used (through ethPerDerivative()), which is a time-weighted exponential moving average. In WstEth a 1-1 peg was assumed between stETH and wstETH in the slippage evaluation, so it couldn't be manipulated. Note that this has been changed in the mitigation of H-06: WstEth derivative assumes a ~1=1 peg of stETH to ETH..

Mitigation review

Execution independent slippage protection has been introduced in unstake() by the mitigation of M-12: No slippage protection on stake() in SafEth.sol. This solves the problem for Reth. (The spot price oracle has also been replaced by a Chainlink price feed, but the issue is mitigated by the new slippage protection alone.)

The 1-1 peg assumption in WstEth has been replaced by a Chainlink price feed, which should be stable and secure enough.

There is no Chainlink price feed for frxEth so SfrxEth.ethPerDerivative() hardcodes a 1-1 peg with ETH, so there is nothing to manipulate (just like previously was the case with WstEth).

Mitigation error: No sanity check on Chainlink response

There are no sanity checks on the Chainlink price feed return data, especially that it is not stale. Here is an overview on this matter. See the error report on this titled "[H-02, H-05, H-06, H-08] mitigation error: No sanity check on Chainlink price feed".

elmutt commented 1 year ago

thanks

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory