Open code423n4 opened 3 years ago
We acknowledge the issue for the protocol's AMM, but if this becomes a large issue in the future, the router is easily upgradeable to include a minimum rate parameter.
Have changed this to confirmed; even though we already were aware of it; we have discussed and are happy to add in a UI-handed arg for minAmount now rather than reactively in the future. Disagree with severity though; this wasn't a problem with V1 at all.
I'll keep high risk as sandwich attacks are very common and risk of getting a bad swap is real.
Handle
tensors
Vulnerability details
Impact
There are no minimum amounts out, or checks that frontrunning/slippage is sufficiently mitigated. This means that anyone with enough capital can force arbitrarily large slippage by sandwiching transactions, close to 100%.
Proof of Concept
https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/Pool.sol#L284
https://github.com/code-423n4/2021-07-spartan/blob/e2555aab44d9760fdd640df9095b7235b70f035e/contracts/Pool.sol#L296
Recommended Mitigation Steps
Add a minimum amount out parameter. The function reverts if the minimum amount isn't obtained.