Increasing Length Array as Loop Variable

Overview

• Severity: High
• Confidence: Medium
• Affected Versions: All

What is the Increasing Length Array as Loop Variable vulnerability?

Solidity, like many programming languages, offers both looping constructs and array data types. It is a relatively common pattern in many languages to use the length of an array as the control variable for a loop, iterating over each entry of the array within the loop body. In the context of Ethereum, unlike many other programmable systems, there is a possibility that a transaction may run out of gas and fail to complete its computation, which may particularly be an issue when processing loops, if the bound of the loop grows too large. Thus, loops that use an array length that can grow but never decrease may be at risk of gas-related denial of service.

Further reading: Solidity Documentation: Array Members

Technical example of vulnerable code

  // SPDX-License-Identifier: Unlicense
  pragma solidity ^0.8.0;

  contract VotingSystem {
      struct Candidate {
          string name;
          uint256 voteCount;
      }

      Candidate[] public candidates;

      mapping(address => bool) public hasVoted;

      // Add a new candidate to the system
      function addCandidate(string calldata _name) external {
          candidates.push(Candidate({name: _name, voteCount: 0}));
      }

      // Vote for a candidate
      function vote(uint256 _candidateIndex) external {
          require(!hasVoted[msg.sender], "You have already voted.");
          require(_candidateIndex < candidates.length, "Invalid candidate index.");

          candidates[_candidateIndex].voteCount += 1;
          hasVoted[msg.sender] = true;
      }

      // Function to tally votes for each candidate (can become stuck due to gas limits)
      function tallyVotes() external view returns (string memory winnerName) {
          uint256 highestVoteCount = 0;
          for (uint256 i = 0; i < candidates.length; i++) {
              if (candidates[i].voteCount > highestVoteCount) {
                  highestVoteCount = candidates[i].voteCount;
                  winnerName = candidates[i].name;
              }
          }
      }
  }

In the example above, the VotingSystem contract implements a way of tallying votes for candidates on-chain; in practice, this could be as part of a Decentralized Autonomous Organization (DAO). The function tallyVotes() loops over all of the candidates recorded in the contract, using the .length property of the candidates array to control the loop; however, this array can only grow in size and cannot decrease, meaning if too many candidates are added to the array, the tally will never succeed due to gas limitations.

Technical example of how to fix the vulnerability

  // SPDX-License-Identifier: Unlicense
  pragma solidity ^0.8.0;

  contract VotingSystemUpdated {
      struct Candidate {
          string name;
          uint256 voteCount;
      }

      Candidate[] public candidates;
      // Track the current leading candidate's index and vote count for efficient access
      uint256 private leadingCandidateIndex;
      uint256 private leadingVoteCount;

      mapping(address => bool) public hasVoted;

      constructor() {
          // Initialize leading candidate index and vote count to an impossible condition
          leadingCandidateIndex = type(uint256).max;
          leadingVoteCount = 0;
      }

      // Add a new candidate to the system
      function addCandidate(string calldata _name) external {
          candidates.push(Candidate({name: _name, voteCount: 0}));
          // Initialize leading candidate if this is the first candidate being added
          if (leadingCandidateIndex == type(uint256).max) {
              leadingCandidateIndex = 0;
              leadingVoteCount = 0;
          }
      }

      // Vote for a candidate
      function vote(uint256 _candidateIndex) external {
          require(!hasVoted[msg.sender], "You have already voted.");
          require(_candidateIndex < candidates.length, "Invalid candidate index.");

          candidates[_candidateIndex].voteCount += 1;
          hasVoted[msg.sender] = true;

          // Update the current leader if this candidate now has more votes
          if (candidates[_candidateIndex].voteCount > leadingVoteCount) {
              leadingCandidateIndex = _candidateIndex;
              leadingVoteCount = candidates[_candidateIndex].voteCount;
          }
      }

      // Function to get the current leading candidate, avoiding the loop
      function getCurrentLeader() external view returns (string memory winnerName, uint256 voteCount) {
          // Ensure there is at least one candidate
          require(leadingCandidateIndex != type(uint256).max, "No candidates available.");
          return (candidates[leadingCandidateIndex].name, leadingVoteCount);
      }
  }

In the revised example, contract VotingSystemUpdated has reworked logic to track the leading candidate when the vote() function is called, rather than using a loop over the candidates array in the previous example, avoiding the possibility of the loop running out of gas.