sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

peanuts - Unbounded loop leading to DoS if the value of latestID is too large #400

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

peanuts

medium

Unbounded loop leading to DoS if the value of latestID is too large

Summary

Unbounded loop leading to DoS by exceeding gas limit if the value of latestID is too large.

Vulnerability Detail

If there are too many vaults deployed, many functions such as claimTokens(), executeDeposits(), rewardsToArray(), rebalanceCheckProtocols(), setTotalUnderlying() will revert due to exceeding the gas limit because the value of latestID will may be too large.

  function claimTokens() public {
    uint256 latestID = controller.latestProtocolId(vaultNumber);
    for (uint i = 0; i < latestID; i++) {
      if (currentAllocations[i] == 0) continue;
      bool claim = controller.claim(vaultNumber, i);
      if (claim) {
        address govToken = controller.getGovToken(vaultNumber, i);
        uint256 tokenBalance = IERC20(govToken).balanceOf(address(this));
        Swap.swapTokensMulti(
          Swap.SwapInOut(tokenBalance, govToken, address(vaultCurrency)),
          controller.getUniswapParams(),
          false
        );
      }
    }
  }

Impact

Unbounded loop leading to DoS if there is too many vaults.

Code Snippet

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

Tool used

Manual Review

Recommendation

Recommend breaking up the loop to accommodate smaller indexes so the protocol will not break when there is too many new vaults.