No slippage check while removing liquidity from Curve Pool

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/Curve2PoolAdapter.sol#L168 https://github.com/code-423n4/2023-11-shellprotocol/blob/main/src/adapters/Curve2PoolAdapter.sol#L170

Vulnerability details

Impact

The hardcoded value of 0 for the _min_amount parameter in the remove_liquidity_one_coin function of the Curve pool can have significant effects on user funds. When users are removing liquidity from the curve pool, the _min_amount parameter represents the minimum amount of the desired coin that the user expects to receive in return for the provided LP tokens. By setting this value to 0, the code exposes users to potential slippage issues, where they may receive fewer coins than anticipated, leading to financial losses.

Proof of Concept

In the function primitiveOutputAmount although user is giving the minimumAmountOut but the issue is that while removing the liquidity to the curve pool the minimum amount out is hardcoded to 0 which can cause loss of funds for the user

else {
            rawOutputAmount = ICurve2Pool(primitive).remove_liquidity_one_coin(rawInputAmount, indexOfOutputAmount, 0);
        }

If you see the above code snippet then in the else statement the code is calling remove_liquidity_one_coin function of curve pool and giving three things as input, first thing is token amount and second thing is Index value of the coin to withdraw and third is _min_amount (Minimum amount of coin to receive)

But the issue is that _min_amount is hardcoded to 0 so user could face slippage issue or MEV attack. If we see the Curve pool documentation which is at here and here then we can see that indeed third index is the amount of coins that the user receive, here is the relevent part from documentation

StableSwap.remove_liquidity_one_coin(_token_amount: uint256, i: int128, _min_amount: uint256)→ uint256
Withdraw a single coin from the pool.

_token_amount: Amount of LP tokens to burn in the withdrawal

i: Index value of the coin to withdraw

_min_amount: Minimum amount of coin to receive

Returns the amount of coin i received.

If the minimum Amount is set to 0 then user could end up loosing his/her funds due to slippage or MEV attack.

Note: kind of similar issue is here

else if (action == ComputeType.Deposit) {
            uint256[2] memory inputAmounts;
            inputAmounts[uint256(int256(indexOfInputAmount))] = rawInputAmount;
            rawOutputAmount = ICurve2Pool(primitive).add_liquidity(inputAmounts, 0);
        }

where while adding liquidity the _min_mint_amount (Minimum amount of LP tokens to mint from the deposit) is hardcoded to 0

Tools Used

Manual Review

Recommended Mitigation Steps

Allow user to input these values

Assessed type

Error

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #87

[c4-judge]

0xA5DF marked the issue as unsatisfactory: Invalid