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

0 stars 0 forks source link

`_harvest` has no slippage protection when swapping `auraBAL` for `AURA` #104

Open code423n4 opened 2 years ago

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#L275

Vulnerability details

Impact

Single swaps of _harvest contains no slippage or deadline, which makes it vulnerable to sandwich attacks, MEV exploits and may lead to significant loss of yield.

Proof of Concept

When using BALANCER_VAULT.swap here and here, there is no slippage protection. Therefore a call to _harvest generating swaps could be exploited for sandwich attacks or other MEV exploits such as JIT.

The scenario would be: A authorized actor calls harvest, leading to a swap of say x auraBAL to BAL/ETH BPT and then y WETH to BAL.

Then while the transaction is in the mempool, it is exploited for example like in https://medium.com/coinmonks/defi-sandwich-attack-explain-776f6f43b2fd

Recommended Mitigation Steps

The easiest mitigation would be to pass a minimum amount of AURA that the swap is supposed to get in harvest. It should not add security issues as callers of harvest are trusted.

An other solution would be to do like here to use Cowswap for example, or any other aggregator.

GalloDaSballo commented 2 years ago

Dup of #155

GalloDaSballo commented 2 years ago

I love how the warden linked my code to integrate cowswap XD

GalloDaSballo commented 2 years ago

Confirmed and mitigated in 2 ways: