No slippage checks can lead to sandwich attacks

[code423n4]

Handle

cmichel

Vulnerability details

Several functions trade without using any slippage protection, their min. return amount is set to 1:

• Controller.setCap: _balance = _converter.convert(_want, _convert, _balance, 1);
• Controller.withdrawAll: _amount = _converter.convert(_want, _convert, _amount, 1);
• NativeStrategyCurve3Crv._swapTokens: _swapTokens(weth, _stableCoin, _remainingWeth, 1);

Combined with the fact that anyone can call Harvester.harvestNextStrategy, it's easy to sandwich-attack these.

A common attack in DeFi is the sandwich attack. Upon observing a trade of asset X for asset Y, an attacker frontruns the victim trade by also buying asset Y, lets the victim execute the trade, and then backruns (executes after) the victim by trading back the amount gained in the first trade. Intuitively, one uses the knowledge that someone’s going to buy an asset, and that this trade will increase its price, to make a profit. The attacker’s plan is to buy this asset cheap, let the victim buy at an increased price, and then sell the received amount again at a higher price afterwards.

Impact

The protocol makes a bad trade and loses tokens. The profit is not maximized.

Recommended Mitigation Steps

Accept a function parameter that can be chosen by the transaction sender, then check that the actually received amount is above this parameter instead of using the hardcoded 1.

harvestNextStrategy may not be called by anyone as they could set the slippage parameter very low. Let the strategist call harvestNextStrategy.

Check if it's feasible to send these transactions directly to a miner (flashbots) such that they are not visible in the public mempool.

[BobbyYaxis]

harvestNextStrategy cannot be called by non-whitelisted addresses

[GalloDaSballo]

Warden findings are correct, but they are not properly articulated. Other findings point to risk of front-running, sandwiching and Single Sided Exposure risk

Downgrading to Low Risk

For Sponsor: The fact you can call the function doesn't prevent the trade from being front-run as once you sign the transaction it is publicly available in the mempool, frontrunner can then write sandwiching txs with the goal of maximing MEV out of your trade