The current system offers no guarantee that a user swap behaved correctly. Although most DEX have internal slippage protection that could be used using the callData of the SwapData struct, it would be better and not very expensive to add a parameter minToAmount to verify internal the result of the swap.
Proof of Concept
Slippage is very important for users as onChain swaps are subject to sandwich attacks and MEV extraction. It should therefore be a requirement for every swap. Furthermore it would offer an additional guarantee that the parameters between the SwapData struct and the callData are coherent.
Recommended Mitigation Steps
toAmount is already computed, so adding a slippage check is just an additional require
Lines of code
https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L48
Vulnerability details
Impact
The current system offers no guarantee that a user swap behaved correctly. Although most DEX have internal slippage protection that could be used using the
callData
of theSwapData
struct, it would be better and not very expensive to add a parameterminToAmount
to verify internal the result of the swap.Proof of Concept
Slippage is very important for users as onChain swaps are subject to sandwich attacks and MEV extraction. It should therefore be a requirement for every swap. Furthermore it would offer an additional guarantee that the parameters between the
SwapData
struct and thecallData
are coherent.Recommended Mitigation Steps
toAmount
is already computed, so adding a slippage check is just an additionalrequire