search

code-423n4 / 2021-04-maple-findings

0 stars 0 forks source link

Slippage should not be allowed to be set to very high values #109

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

janbro

Vulnerability details

Summary

setMaxSwapSlippage should not allow slippage to be set to very high values (e.g. 100% is a permitted value).

Risk Rating

Low

Vulnerability Details

MapleGlobals.sol Line 140: function setMaxSwapSlippage(uint256 newMaxSlippage) external isGovernor { _checkPercentageRange(newMaxSlippage); maxSwapSlippage = newMaxSlippage; emit GlobalsParamSet("MAX_SWAP_SLIPPAGE", newMaxSlippage); }

Users should be protected by setting an upper bound for the max slippage under 100%. High slippage values increases exposure to sandwich attacks during liquidation of collateral. No utility is added by allowing these values and risk is only increased.

Impact

See https://cmichel.io/de-fi-sandwich-attacks/

Tools Used

Manual code review

Recommended Mitigation Steps

Implement a limit on the max value passed to setMaxSwapSlippage.

lucas-manuel commented 3 years ago

This was intentional, not a bug.

Arachnid commented 3 years ago

@lucas-manuel Can you elaborate? Why should 100% be a permitted value?

lucas-manuel commented 3 years ago

@Arachnid We have this in place just in case we run into a liquidity issue down the road and just need to recover as much as we can during a liquidation. It's important to have this as a worst-case scenario option because all loans must be closed before a PD can get their StakeLockerFDTs back (they can only get them back when a Pool is deactivated)