Core Contracts

Description & Impact

_withdrawFromVault() is called internally by _ensureLiquidity() and _rebalanceLiquidity() which are in turn called by withdraw(), borrow(), deposit().
The _withdrawFromVault() function is called whenever there is shortage of funds in the pool itself and funds need to be withdrawn from the Curve Vault:

The fourth parameter in the call to curveVault.withdraw() is hard-coded to 0 which refers to maxLoss or Maximum acceptable loss in basis points as mentioned here.

As we can see from Curve's docs here, there is a withdrawal fee applicable on pulling liquidity out of the vaults:

Newer pools automatically claim admin fees throughout the week when users withdraw their liquidity from the pools.

Hence, depending on how the curveVault.withdraw() is implemented, one of two things may happen:

1. If Curve deducts the fee from the withdrawal amount first and then compares with maxLoss, then hard-coded 0 will always lead to a revert.

2. If comparison with maxLoss is done before fee deduction then the amount returned will never be equal to the param amount passed. It will be lesser since fee is deducted. This means the assumptions made by _ensureLiquidity() on L763-L765 and _rebalanceLiquidity() on L787-L789 that the amount being pulled from CurveVault equals that asked for will turn out to be false. Finally, borrow() and withdraw(), which have assumed this whole amount is credited after pulling funds from CurveVault, will revert due to shortage of funds.

This is apart from any rounding which may happen at Curve's side while calculating withdrawal amount vs shares and also any slippage which may occur.

Mitigation

Pass an amount which is a bit higher than required so that even after fee deduction, there is enough. Also, verify the amount created by checking the difference in balances before & after the call to _withdrawFromVault().