search

code-423n4 / 2024-03-abracadabra-money-findings

9 stars 7 forks source link

Adjusting "_I_" will create a sandwich opportunity because of price changes #171

Open c4-bot-7 opened 6 months ago

c4-bot-7 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L470-#L510 https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L244-#L265

Vulnerability details

Vulnerability details

Because MagicLP is forked from DODO, we will check DODO documentation to understand this contract. From DODO documentation, the "I" is the "i" value in here and it is directly related with the output amount a trader will receive when selling a quote/base token:

img

Adjusting the value of "I" directly by calling setParameters() function will influence the price. This can be exploited by a MEV bot, simply by trading just before the setParameters() function and exiting right after the price change. The profit gained from this operation essentially represents potential losses for the liquidity providers who supplied liquidity to the pool.

Impact

Since the price will change, the MEV bot can simply sandwich the tx and get profit.

Another note on this is that even though the adjustPrice called by onlyImplementationOwner without getting frontrunned, it still creates a big price difference which requires immediate arbitrages. Usually these type of parameter changes that impacts the trades are setted by time via ramping to mitigate the unfair advantages that it can occur during the price update.

Tools Used

Manual review

Recommended Mitigation Steps

Acknowledge the issue and use private RPC's to eliminate front-running or slowly ramp up the "I" so that the arbitrage opportunity is fair

Assessed type

MEV

c4-pre-sort commented 5 months ago

141345 marked the issue as primary issue

c4-pre-sort commented 5 months ago

141345 marked the issue as sufficient quality report

141345 commented 5 months ago

admin func to change "I" needs time lock

c4-sponsor commented 5 months ago

0xCalibur (sponsor) acknowledged

0xCalibur commented 5 months ago

Fixed in main https://github.com/Abracadabra-money/abracadabra-money-contracts/

We now have protocol own pool and can disable trading for our own pools, changing the parameters and make sure the pool is in good state before enabling it back.

c4-judge commented 5 months ago

thereksfour marked the issue as satisfactory

c4-judge commented 5 months ago

thereksfour marked the issue as selected for report