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

0 stars 0 forks source link

_harvest rewards can be stolen because it doesn't implement any slippage bounds #131

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#L219-L282

Vulnerability details

Impact

Harvested funds stolen

Proof of Concept

_harvest does not implement any kind of minimum out when calling the 3 consecutive swaps (L249, L263 and L275) to get from auraBal to Aura. An attacker could easily sandwich the least liquid pool and steal all the harvested funds

Tools Used

Recommended Mitigation Steps

All balancer pools have a TWAP built into them. For each swap first pull the TWAP value and implement a slippage calculation based on that. Keep in mind that TWAP is updated before the state change of the previous swap therefore the slippage value chosen should be wider to account for it to keep failed transactions to a minimum

KenzoAgada commented 2 years ago

Duplicate of #155