code-423n4 / 2022-06-badger-findings

0 stars 0 forks source link

Loss of yield can occur due to not specifying `minAmountsOut` when exiting `BAL/ETH pool` #130

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L259

Vulnerability details

Impact

When exiting the BAL/ETH pool, due to not specifying anything for minAmountsOut an attacker can frontrun the transaction and cause a large change in price in the pool. This in turn leads to a large impermanent loss which is realised when the strategy burns its liquidity tokens.

I believe this a high severity issue because this can cause a large substantial loss of yield thereby rendering the whole purpose of the entire strategy meaningless.

Proof of Concept

  1. Strategy makes transaction to exit BAL/ETH pool which goes into the mempool
  2. An attacker sees the transaction and makes a very large flash swap on the pool, almost emptying all of ETH or BAL
  3. Strategy exits pool and receives significantly less value due to impermanet loss

Tools Used

VS Code

Recommended Mitigation Steps

Use queryExit() from BalancerHelpers to specify a value for minAmountsIn as shown here

Make sure to do this before the transaction containing _harvest() is run

KenzoAgada commented 2 years ago

Duplicate of #155