BaseVaultAdaptor withdraws too much

[code423n4]

Handle

cmichel

Vulnerability details

Vulnerability Details

The two BaseVaultAdaptor.withdraw functions first check if the amount is already in the adapter, if not they withdraw the full amount again from the vault.

If one wants to minimize withdrawals from the yield-earning strategies, it'd be better to only withdraw the difference of amount and what's already in the vault adaptor from the vault strategies.

amount = _withdraw(calculateShare(amount), msg.sender);
// could be calcShare(amount - token.balanceOf(this))
amount = _withdraw(calcShare(amount.sub(token.balanceOf(this))), msg.sender);

Recommended Mitigation Steps

Withdraw the difference from vault to adopter and then withdraw the full adaptor amount to the recipient.

[kitty-the-kat]

This would be the opposite of what we want to achieve - we want to keep down prices for small users, which means that we want to always keep a reserve in the vault Adapter from which they can withdraw. If we empty out the vaultadapter by letting larger fish empty them out the cost of withdrawals would drastically go up for the smaller fish

[ghoul-sol]

Works as expected. Invalid.