search

sherlock-audit / 2024-06-makerdao-endgame-judging

5 stars 3 forks source link

0x73696d616f - MEV bots or regular users will frontrun the `univ2-pool-migrator` script and cause loss of funds for Maker #12

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

0x73696d616f

Medium

MEV bots or regular users will frontrun the univ2-pool-migrator script and cause loss of funds for Maker

Summary

The script UniV2PoolMigratorInit burns the lp tokens of the old $DAI $MKR pair without checking minimum amounts or setting a deadline. Thus, mev bots or regular users may frontrun the spell with swaps, modifying the reserves and causing Maker to take a significant loss. The sensitiveness of burning tokens to the reserves can be confirmed by the fact that the UniswapV2Router specifically asks for minimum amounts out and deadline arguments and the $DAI $MKR UniswapV2 pool presents significant price variation at times.

Root Cause

In UniV2PoolMigratorInit.sol, the lps from the $DAI $MKR pair are burned and the received amounts are not checked against a threshold deviation nor a deadline is enforced.

Internal pre-conditions

None.

External pre-conditions

  1. Regular price movement that results in Maker taking a loss, which is likely as the pair is volatile or MEV can be performed.

Attack Path

  1. Maker executes the UniV2PoolMigratorInit script.
  2. Users or a MEV bot frontruns the script and modifies the price.
  3. The script is executed and Maker takes a loss and receives a very different number of $DAI and $MKR tokens.

Impact

Maker takes a loss and receives less total value than expected. The amount of tokens received can vary significantly as the price is very volatile, as shown in the POC section, in just 1 minuted it changed 0.1% up to 5.38% in 1 day.

PoC

Here can be seen that at the time of writing, the price is volatile and changed 0,1%, 1.1%, 1.84%, 5.38% in the last 1 minute, 5 minutes, 6 hours, 24 hours. Thus, there is a big uncertainty in the amount of tokens actually received.

Mitigation

Check the amounts received and ideally place a deadline to mitigate this loss, similarly to the validations performed in the router.

telome commented 3 months ago

The UniswapV2Router specifically asks for minimum amounts so the depositor/withdrawer can know at what price they do their action, while for the migrator this doesn't matter as it maintains the same price, just moves to another pool. Also see note 7.1 here - https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/univ2-pool-migrator/audit/ChainSecurity_MakerDAO_UniV2_Migration_audit.pdf