Swap routes are hardcoded for pools that do not exist yet.

[code423n4]

Lines of code

https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L229 https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L240 https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L251 https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L265

Vulnerability details

Impact

The route for swapping auraBAL to AURA is hardcoded and does not allow any flexibility.

Proof of Concept

The route for this swap is hardcoded to auraBAL -> BAL/ETH BPT -> WETH -> AURA, with specific pool IDs. This seems to be done for the sake of simplicity and to use the interface of BaseStrategy which does not allow for any argument to be passed when calling harvest. But it can turn out to be a pain and lead to a loss of yield.

At the time of writing there is no liquidity in the auraBAL/BAL/ETH BPT, and it could happen that the pool with the most liquidity changes over time. It could also happen punctually that there is a more interesting route than the current one, in which case the contract wouldn't be able to use it.

In the worst case, if for example the pool for swapping auraBAL to BAL/ETH BPT is unused or has very few liquidity, which could happen as at the time of writing it does not hold any liquidity, and auraBAL has just been released, then this implementation becomes useless and harvesting would waste the earned `auraBAL.

Recommended Mitigation Steps

• Why not enabling the caller of harvest, who should already be whitelist hence trusted to use an aggregator like CowSwap or 1Inch for the swap ? The yield would be better for users, and it does not add a lot of overhead.

• If the issue is to reuse the BaseStrategy interface, then it could be done by doing the swap in an other function. Then the flow would be:

  • harvest claims auraBAL and report the amount of the last swap done.
  • In an other function swapAuraBAL do the swap using a DEX aggregator and an authorized keeper and store the result to report it during the next harvest.

[GalloDaSballo]

Disagree, they exist at time of the contest starting