Medium: Consider alternative price feed + ensure _minLockPeriod > 0 to prevent flash loan attacks

[code423n4]

Handle

hickuphh3

Vulnerability details

Impact

It is critical to ensure that _minLockPeriod > 0 because it is immutable and cannot be changed once set. A zero minLockPeriod will allow for flash loan attacks to occur. Vaults utilising the nonUST strategy are especially susceptible to this attack vector since the strategy utilises the spot price of the pool to calculate the total asset value.

Proof of Concept

Assume the vault’s underlying token is MIM, and the curve pool to be used is the MIM-UST pool. Further assume that both the vault and the strategy holds substantial funds in MIM and UST respectively.

1. Flash loan MIM from the Uniswap V3 MIM-USDC pool (currently has ~$3.5M in MIM at the time of writing).
2. Convert half of the loaned MIM to UST to inflate and deflate their prices respectively.
3. Deposit the other half of the loaned MIM into the vault. We expect curvePool.get_dy_underlying(ustI, underlyingI, ustAssets); to return a smaller amount than expected because of the previous step. As a result, the attacker is allocated more shares than expected.
4. Exchange UST back to MIM, bringing back the spot price of MIM-UST to a normal level.
5. Withdraw funds from the vault. The number of shares to be deducted is lower as a result of (4), with the profit being accounted for as yield.
6. Claim yield and repay the flash loan.

Recommended Mitigation Steps

Ensure that _minLockPeriod is non-zero in the constructor. Also, given how manipulatable the spot price of the pool can be, it would be wise to consider an alternative price feed.

// in Vault#constructor
require(_minLockPeriod > 0, 'zero minLockPeriod');

[naps62]

duplicate of #65

[dmvt]

65 is a duplicate of this given that this issue's description is better