It lacks slippage control when swapping tokens

[code423n4]

Lines of code

https://github.com/Badger-Finance/vested-aura/blob/v0.0.2/contracts/MyStrategy.sol#L249 https://github.com/Badger-Finance/vested-aura/blob/v0.0.2/contracts/MyStrategy.sol#L257-L263 https://github.com/Badger-Finance/vested-aura/blob/v0.0.2/contracts/MyStrategy.sol#L275

Vulnerability details

Impact

In balancer document:

In the above example code, we set our token_BAL limit to 0, which means we are willing to accept 100% slippage on our trade. That is generally a very bad idea

It lacks slippage control when calling BALANCER_VAULT.swap, making it suffer from 100% slippage and front-running attack.

Proof of Concept

The third parameter of BALANCER_VAULT.swap is 0:

https://github.com/Badger-Finance/vested-aura/blob/v0.0.2/contracts/MyStrategy.sol#L249

            uint256 balEthBptEarned = BALANCER_VAULT.swap(singleSwap, fundManagement, 0, type(uint256).max);

https://github.com/Badger-Finance/vested-aura/blob/v0.0.2/contracts/MyStrategy.sol#L275

            harvested[0].amount = BALANCER_VAULT.swap(singleSwap, fundManagement, 0, type(uint256).max);

And the minAmountsOut is empty:

https://github.com/Badger-Finance/vested-aura/blob/v0.0.2/contracts/MyStrategy.sol#L257-L263

            IBalancerVault.ExitPoolRequest memory exitPoolRequest = IBalancerVault.ExitPoolRequest({
                assets: assets,
                minAmountsOut: new uint256[](2),
                userData: abi.encode(ExitKind.EXACT_BPT_IN_FOR_ONE_TOKEN_OUT, balEthBptEarned, BPT_WETH_INDEX),
                toInternalBalance: false
            });
            BALANCER_VAULT.exitPool(BAL_ETH_POOL_ID, address(this), payable(address(this)), exitPoolRequest);

Tools Used

None

Recommended Mitigation Steps

Consider setting a limit value for BALANCER_VAULT.swap and minAmountsOut.

minAmountsOut good practice:

A good practice would be to user queryExit in BalancerHelpers to find the current amounts of tokens you would get for your BPT, and then account for some possible slippage.

[GalloDaSballo]

We use flashbots, advice is still vulnerable. Because of leak of value I disagree with High

[KenzoAgada]

Duplicate of #155