Possible sandwich-attack when treasury converts tokens

[code423n4]

Handle

@cmichelio

Vulnerability details

Vulnerability Details

As the docs say:

If the MapleTreasury collects fees from a Loan drawdown that was performed with WBTC, it can swap WBTC to USDC so that it can distribute those funds to token holders.

When this trade is observed, it can be sandwich-attacked by arbitrage bots observing the mem pool.

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 default max slippage (_globals.maxSwapSlippage()) is set to 10% and is too high which makes attacks profitable compared to the swap fees the attacker would have to pay:

maxSwapSlippage      = 1000;       // 10 %

Impact

The attacker's profit is the treasury's loss and would result in the Maple treasury receiving fewer tokens than at market price.

Recommended Mitigation Steps

Set a max slippage to at most 1% and add a second parameter to convertERC20 that allows specifying the amount of tokens to trade at once, instead of the whole amount. A lower trade size (chosen according to the swap pool's reserves) makes sandwich attacks not profitable.

[lucas-manuel]

We'll address this with a more custom function for converting

[lucas-manuel]

Spoke internally and we do not view this as a bug, not addressing.

[Arachnid]

Per code-423n4 guidelines, problems that are ecosystem-wide like frontrunning/sandwiching are generally not notable. The default slippage being very high warrants Note, though.

[Arachnid]

Duplicate of #106.