No slippage control on any sell (eg: Curves.sellCurvesToken) makes sells vulnerable to sandwich attacks

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L282-L293

Vulnerability details

Impact

Users have no slippage control or protection on sells. Therefore, they cannot prevent a malicious user who owns the same token from selling a massive amount of tokens (tx1) before their sell tx executes (tx2), causing them to receive much less ETH than expected. The same malicious user can then buy their tokens back at a reduced cost (tx3) after the first two transactions execute within the same block.

Proof of Concept

1. User A decides to sell some tokens and calls sellCurvesToken expecting a certain amount of ETH in return.
2. Malicious user M is monitoring the mempool for large sell transactions and submits a sellCurvesToken tx of their own with higher gas than user A.
3. M also submits a tx to buy back the token they sold in #2 at a reduced cost with lower gas than user A.
4. User A receives less ETH than expected after accounting for fees.

Tools Used

Manual review

Recommended Mitigation Steps

Along with the typical "run a private mempool" which may not be very practical, some more interesting options are:

• Add a minimum expected amount input param to the sell functions, reverting if the price is less than this expected amount
• Rework the buy and sell functions adding full slippage controls across the protocol
• Add a timelock to recent buys - something as simple as requiring 1 block between buy and sell transactions can prevent this scenario

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #13

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid