No slippage tolerance amount for curve swaps

Summary

There are a few places where curve swaps are executed without slippage tolerance, which could lead to malicious sandwiching.

Vulnerability Detail

For example, in SdtRewardReceiver.sol in _withdrawRewards:

We have:

_poolCvgSDT.exchange(0, 1, rewardAmount, _poolCvgSDT.get_dy(0, 1, rewardAmount), receiver);

(For context rewardAmount is the SDT reward amount and we want to convert it to CvgSDT). The min amount out is calculated fully on chain, which means that this swap could be prone to sandwiching.

There are a few other swaps that are executed poorly but not as poorly. For example in SdtUtilities, in convertAndStakeSdAsset, we have:

                if (crvPoolPlain.get_dy(0, 1, _assetAmount) > _assetAmount) {
                    crvPoolPlain.exchange(0, 1, _assetAmount, _assetAmount, address(this));
                }

This is still prone to sandwiching, as if crvPoolPlain.get_dy(0, 1, _assetAmount) is much more than _assetAmount, the sandwicher can make it so that the user gets only _assetAmount out instead of the higher amount they should get.

Impact

User's swaps can get maliciously sandwiched, leading to loss of money

Code Snippet

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/Staking/StakeDAO/SdtRewardReceiver.sol#L235

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/utils/SdtUtilities.sol#L151

https://github.com/sherlock-audit/2023-11-convergence/blob/main/sherlock-cvg/contracts/utils/SdtUtilities.sol#L95

Tool used

Manual Review

Recommendation

Allow setting a min amount out that the user must get out

Duplicate of #180

sherlock-audit / 2023-11-convergence-judging

detectiveking - No slippage tolerance amount for curve swaps #186