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

0 stars 0 forks source link

_harvest() performs market swaps without slippage control and is a subject to sandwich attacks #155

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

As a result trades happen at a manipulated price and end up receiving fewer tokens than current market price dictates.

Placing severity to medium as impact here is a partial fund loss conditional only on big enough asset amount to be swapped: sandwich attacks are common and can be counted to happen almost always as long as economic viability is present.

Proof of Concept

There is no slippage control of any kind in reward token swapping:

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

Recommended Mitigation Steps

Consider adding slippage argument to the _harvest, so it can be tuned each time according to the current market conditions.

GalloDaSballo commented 2 years ago

We use private transactions

jack-the-pug commented 2 years ago

Dup of #104