Should not rely solely on user provided slippage parameter

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/CurveTricryptoAdapter.sol#L178-L236 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/Curve2PoolAdapter.sol#L142-L184

Vulnerability details

The curve adapters should not rely entirely on the user provided minimumOutputAmount value to determine slippage allowance in the primitiveOutputAmount() function; see here and here.

A user could either enter an incorrect/misguided value which is too low or if their transaction has been pending too long, their original estimate may be based on stale prices. In either case they would stand to lose a lot of money.

An incorrectly entered value is particularly poignant in the case of CurveTricryptoAdapter.sol whose three tokens use three different decimals; i.e. 6, 8 & 18 easily resulting in wrong inputs.

This would make the check in primitiveOutputAmount() inadequate and user vulnerable to MEV attack. If the user places far too low a value (or no value at all) they can stand to lose a substantial amount of their funds.

if (uint256(minimumOutputAmount) > outputAmount) revert SLIPPAGE_LIMIT_EXCEEDED();

Impact

If a user makes a very large deposit with minimumOutputAmount set too low; while the transaction is pending in the mempool; a malicious user can craft a sandwich attack to take advantage. Attacker would first execute a transaction ahead of the user's (front-run), which increases the price of the outputToken. The user's transaction then executes at this inflated price. Then, the attacker executes another transaction after the user's (back-running), selling outputToken at the inflated price. This sequence exploits the user's low minimumOutputAmount setting, causing them to receive less than they might have expected under normal market conditions.

Tools Used

Foundry Manual Review

Recommended Mitigation Steps

Add a check to the two Curve adapters which checks user's provided value against Curve's Moving Average using the oracle_price function.

See: https://resources.curve.fi/factory-pools/understanding-oracles/#exponential-moving-average

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #214

[c4-pre-sort]

raymondfam marked the issue as duplicate of #277

[c4-judge]

0xA5DF changed the severity to QA (Quality Assurance)

[c4-judge]

0xA5DF marked the issue as grade-c

[0xA5DF]

Low quantity