on shortage vault will not pull all the available funds

Summary

On shortage Vault#pullFunds method should pull assets from all underlying protocols until the desired value is less then the vault balance. But in the current implementation it can stop pulling prematurely.

Vulnerability Detail

When Vault#pullFunds is withdrawing from underlying protocols it stops when reaches an amount to withdraw that is lower then the minimum pull value.

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Vault.sol#L122

Note: the minimum pull value represented as minimumPull non immutable variable is once set in the constructor but has no method to be reset to another value in the case it nets to be reset.

Impact

By not keeping withdrawing from all the underlying vault protocols a vault can't pull funds even if it has enough underlying balance.

Code Snippet

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Vault.sol#L122

Tool used

Manual Review

Recommendation

Instead of just break from the withdrawing loop skip the current and continue to the next vault protocol

  function pullFunds(uint256 _value) internal {
    uint256 latestID = controller.latestProtocolId(vaultNumber);
    for (uint i = 0; i < latestID; i++) {
      if (currentAllocations[i] == 0) continue;

      uint256 shortage = _value - vaultCurrency.balanceOf(address(this));
      uint256 balanceProtocol = balanceUnderlying(i);

      uint256 amountToWithdraw = shortage > balanceProtocol ? balanceProtocol : shortage;
      savedTotalUnderlying -= amountToWithdraw;

-     if (amountToWithdraw < minimumPull) break;
+     if (amountToWithdraw < minimumPull) continue; 

      withdrawFromProtocol(i, amountToWithdraw);

      if (_value <= vaultCurrency.balanceOf(address(this))) break;
    }
  }

sherlock-audit / 2023-01-derby-judging

imare - on shortage vault will not pull all the available funds #371