code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Autocompounding auraBAL rewards into AURA requires multiple swaps (auraBAL -> BAL/ETH BPT -> WETH -> AURA) within MyStrategy._harvest.

The swaps are at risk of being front-run / sandwiched, resulting in a loss of funds.

Since MEV is very prominent I think the chance of that happening is pretty high.

Proof of Concept

MyStrategy.sol#L249

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

MyStrategy.sol#L259

IBalancerVault.ExitPoolRequest memory exitPoolRequest = IBalancerVault.ExitPoolRequest({
    assets: assets,
    minAmountsOut: new uint256[](2), // @audit-info missing slippage protection
    userData: abi.encode(ExitKind.EXACT_BPT_IN_FOR_ONE_TOKEN_OUT, balEthBptEarned, BPT_WETH_INDEX),
    toInternalBalance: false
});

MyStrategy.sol#L275

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

Tools Used

Manual review

Recommended mitigation steps

Consider adding slippage checks during swapping.

See https://dev.balancer.fi/resources/joins-and-exits/pool-exits#minamountsout for slippage protection info.

GalloDaSballo commented 2 years ago

We use private harvest, the recommended mitigation is ineffective

KenzoAgada commented 2 years ago

Duplicate of #155

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