code-423n4 / 2021-09-bvecvx-findings

0 stars 0 forks source link

Swap conversion is susceptible to MEV flashbots #34

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

hickuphh3

Vulnerability details

Impact

In veCVXStrategy, the cvxCRV -> ETH -> CVX conversion via sushiswap is done with 0 minAmountOut, making it susceptible to sandwich attacks / MEV flashbots. This is also true for UniSwapper inherited by StrategyCvxHelper.

Recommended Mitigation Steps

  1. veCVXStrategy

    Ideally, the harvest() function should take in a minAmountOut parameter, but this breaks the Yearn architecture used. Using TWAPs / price oracles might alleviate the problem, but results in higher gas usage, and with multiple hops involved, may not be feasible.

    A simpler approach would be to have a configurable storage variable minAmountOut. Its value can then be adjusted such that harvesting can be done infrequently to save gas.

  2. UniSwapper

    Ideally, each path registered in the TokenSwapPathRegistry should also have a minAmount mapping, that can be fetched together with the path.

GalloDaSballo commented 2 years ago

Agree with finding, we use private txs for harvests