sherlock-audit / 2022-11-dodo-judging

0 stars 2 forks source link

__141345__ - `externalSwap()` need slippage control #68

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

141345

medium

externalSwap() need slippage control

Summary

There is no slippage control in externalSwap(). Users could lose fund due to high slippage.

Vulnerability Detail

externalSwap() only checks for whitelist contracts, but not slippage. Currently it might assume that slippage is included in callDataConcat. But there is no guarantee that proper parameters is in `callDataConcat, and no sanity check for it. And the dependence on external protocols might be error prone.

Impact

Users could lose fund due to high slippage.

Code Snippet

https://github.com/sherlock-audit/2022-11-dodo/blob/main/contracts/SmartRoute/DODORouteProxy.sol#L164-L177

Tool used

Manual Review

Recommendation

Add the following check for slippage, just like in mixSwap() and dodoMutliSwap():

    require(minReturnAmount > 0, "DODORouteProxy: RETURN_AMOUNT_ZERO");

Duplicate of #77