hyh - New Beta Finance pool cannot be operated, freezing the rebalancing

[sherlock-admin]

hyh

medium

New Beta Finance pool cannot be operated, freezing the rebalancing

Summary

When there is no shares issued for the pool yet, Beta Finance balanceUnderlying() will be reverted with division by zero, blocking Vault rebalancing.

Vulnerability Detail

If new Beta Finance pool is added to the Vault it will block the rebalancing as BetaProvider's balanceUnderlying() reverts when shares supply is zero.

Impact

Rebalancing will be frozen until this Beta pool be initiated somehow else. Vault cannot blacklist this Beta strategy as blacklistProtocol() relies on balanceUnderlying() as well.

Code Snippet

Beta balanceUnderlying() calculates balance by itself, but do not control for zero supply case:

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/BetaProvider.sol#L77-L92

  /// @notice Get balance from address in underlying token
  /// @dev balance = poolvalue * shares / totalsupply
  /// @param _address Address to request balance from, most likely an Vault
  /// @param _bToken Address of protocol LP Token eg cUSDC
  /// @return balance in underlying token
  function balanceUnderlying(
    address _address,
    address _bToken
  ) public view override returns (uint256) {
    uint256 balanceShares = balance(_address, _bToken);
    uint256 supply = IBeta(_bToken).totalSupply();
    uint256 totalLoanable = IBeta(_bToken).totalLoanable();
    uint256 totalLoan = IBeta(_bToken).totalLoan();

    return (balanceShares * (totalLoanable + totalLoan)) / supply;
  }

totalSupply() == 0 is a valid case for new pools.

balanceUnderlying() is used in Vault rebalancing:

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

  function rebalanceCheckProtocols(
    uint256 _newTotalUnderlying
  ) internal returns (uint256[] memory) {
    uint256[] memory protocolToDeposit = new uint[](controller.latestProtocolId(vaultNumber));
    uint256 latestID = controller.latestProtocolId(vaultNumber);
    for (uint i = 0; i < latestID; i++) {
      bool isBlacklisted = controller.getProtocolBlacklist(vaultNumber, i);

      storePriceAndRewards(_newTotalUnderlying, i);

      if (isBlacklisted) continue;
      setAllocation(i);

      int256 amountToProtocol = calcAmountToProtocol(_newTotalUnderlying, i);
      uint256 currentBalance = balanceUnderlying(i);

      int256 amountToDeposit = amountToProtocol - int(currentBalance);
      uint256 amountToWithdraw = amountToDeposit < 0 ? currentBalance - uint(amountToProtocol) : 0;

      if (amountToDeposit > marginScale) protocolToDeposit[i] = uint256(amountToDeposit);
      if (amountToWithdraw > uint(marginScale) || currentAllocations[i] == 0)
        withdrawFromProtocol(i, amountToWithdraw);
    }

    return protocolToDeposit;
  }

It looks like strategy removal will be impossible also as it queries balanceUnderlying() and similarly be reverted:

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

  function blacklistProtocol(uint256 _protocolNum) external onlyGuardian {
    uint256 balanceProtocol = balanceUnderlying(_protocolNum);
    currentAllocations[_protocolNum] = 0;
    controller.setProtocolBlacklist(vaultNumber, _protocolNum);
    savedTotalUnderlying -= balanceProtocol;
    withdrawFromProtocol(_protocolNum, balanceProtocol);
  }

Tool used

Manual Review

Recommendation

Since it is more low level implementation for Beta, it is needed to control for corner cases in BetaProvider:

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/Providers/BetaProvider.sol#L77-L105

  /// @notice Get balance from address in underlying token
  /// @dev balance = poolvalue * shares / totalsupply
  /// @param _address Address to request balance from, most likely an Vault
  /// @param _bToken Address of protocol LP Token eg cUSDC
  /// @return balance in underlying token
  function balanceUnderlying(
    address _address,
    address _bToken
  ) public view override returns (uint256) {
    uint256 balanceShares = balance(_address, _bToken);
    uint256 supply = IBeta(_bToken).totalSupply();
    uint256 totalLoanable = IBeta(_bToken).totalLoanable();
    uint256 totalLoan = IBeta(_bToken).totalLoan();
+   if (supply == 0) return 0;

    return (balanceShares * (totalLoanable + totalLoan)) / supply;
  }

  /// @notice Calculates how many shares are equal to the amount
  /// @dev shares = totalsupply * balance / poolvalue
  /// @param _amount Amount in underyling token e.g USDC
  /// @param _bToken Address of protocol LP Token eg cUSDC
  /// @return number of shares i.e LP tokens
  function calcShares(uint256 _amount, address _bToken) external view override returns (uint256) {
    uint256 supply = IBeta(_bToken).totalSupply();
    uint256 totalLoanable = IBeta(_bToken).totalLoanable();
    uint256 totalLoan = IBeta(_bToken).totalLoan();
+   if (totalLoanable + totalLoan == 0) return 0;

    return (_amount * supply) / (totalLoanable + totalLoan);
  }

[mister0y]

Not medium severity because it can only cause some functions to revert while there are no funds in the underlying protocol

[hrishibhat]

Considering this issue as low based on the Sponsor comment