savedTotalUnderlying may be unexpectedly reduced

Summary

The total underlying of the vault may be unexpectedly reduced

Vulnerability Detail

Withdrawing the underlying protocol may not be successful when the vault is short of exiting the protocol, but the total underlying in the vault may unexpectedly decrease

Impact

if amountToWithdraw < minimumPull, It will jump out of the current loop. At this time, no bottom layer is extracted, but savedTotalUnderlying will be reduced. Vault.sol#L111-L127

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

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

    if (amountToWithdraw < minimumPull) break;
    withdrawFromProtocol(i, amountToWithdraw);

Code Snippet

see impact

Tool used

Manual Review

Recommendation

  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;
      withdrawFromProtocol(i, amountToWithdraw);
+     savedTotalUnderlying -= amountToWithdraw;

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

Duplicate of #363

sherlock-audit / 2023-01-derby-judging

atrixs - savedTotalUnderlying may be unexpectedly reduced #397