Frontrunning in UniswapHandler calls to UniswapV2Router

[code423n4]

Handle

thank_you

Vulnerability details

Impact

UniswapHandler utilizes UniswapV2Router to swap, add liquidity, and remove liquidity with the UniswapV2Pair contract. In order to utilize these functionalities, UniswapHandler must call various UniswapV2Router methods.

• addLiquidity
• removeLiquidity
• swapExactTokensForTokens (swaps for both DAI and Malt)

In all three methods, UniswapV2Router requires the callee to provide input arguments that define how much the amount out minimum UniswapHandler will allow for a trade. This argument is designed to prevent slippage and more importantly, sandwich attacks.

UniswapHandler correctly handles price slippage when calling addLiquidity. However, that is not the case for removeLiquidity and swapExactTokensForTokens here and here. For both methods, 0 is passed in as the amount out minimum allowed for a trade. This allows for anyone watching the mempool to sandwich attack UniswapHandler (or any contract that calls UniswapHandler) in such a way that allows the hacker to profit off of a guaranteed trade.

How does this work? Let's assume UniswapHandler makes a call to UniswapV2Router#swapExactTokensForTokens to trade DAI for Malt. Any hacker who watches the mempool and sees this transaction can immediately buy as much Malt as they want. This raises the price of Malt. Since UniswapHandler is willing to accept any amount out minimum (the number is set to zero), then the UniswapHandler will always trade DAI for Malt. This second transaction raises the price of Malt even further. Finally, the hacker trades their Malt for DAI, receiving a profit due to the artificially inflated price of Malt from the sandwich attack.

It's important to note that anyone has access to the UniswapV2Router contract. There are no known ACL controls on UniswapV2Router. This sandwich attack can impact even the buyMalt function.

The following functions when called are vulnerable to frontrunning attacks:

• UniswapHandler#buyMalt
• UniswapHandler#sellMalt
• UniswapHandler#removeLiquidity

And by extension the following contract functions since they also call the UniswapHandler function calls:

• Bonding#unbondAndBreak
• LiquidityExtension#purchaseAndBurn
• RewardReinvestor#splitReinvest
• StabilizerNode#stabilize
• SwingTrader#buyMalt

Proof of Concept

Refer to the impact section for affected code and links to the appropriate LoC.

Tools Used

N/A

Recommended Mitigation Steps

The UniswapV2Router and UniswapV2Pair contract should allow only the UniswapHandler contract to call either contract. In addition, price slippage checks should be implemented whenever removing liquidity or swapping tokens. This ensures that a frontrunning attack can't occur.

Anything Else We Should Know

I wish I had more time to work on this bug but unfortunately I have several current clients who require significant time from me. I'm happy to pursue this beyond the initial submission, in particular building a concrete PoC. I think the most important takeaway from this bug find is that anyone can purchase Malt at any time and anyone can manipulate the Malt reserve. This in turn impacts other functionalities that rely on the Malt reserve to make price/token calculations such as exiting an auction early or reinvesting rewards.

[GalloDaSballo]

Because transactions sit on the mempool (which is public, hence accessible by anyone), they can be frontrun, because of this swapping protocols (uniswap in this case) offer slippage checks.
Setting the slippage checks allows a frontrunner to squeeze the maximum amount of value possible (sometimes the whole amount).

Because this applies to a leak of value, I believe medium severity to be correct