Calls in Loop

Overview

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

What is the Calls in Loop vulnerability?

Solidity, like many other programming languages, allows for loop constructs in smart contracts. In addition, the Ethereum Virtual Machine supports making external calls from within a function, which can call other contracts. If external calls are used inside of a loop construct, it is possible that one of the calls may revert and cause the entire function enclosing the loop to revert as well, potentially leading to permanent denial-of-service.

Further reading: Solidity Documentation: Control Structures

Technical example of vulnerable code

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

  contract RewardDistributor {
      address owner;

      constructor() {
          owner = msg.sender;
      }

      modifier onlyOwner() {
          require(msg.sender == owner, "Not the owner");
          _;
      }

      // Function to distribute rewards to a list of addresses
      function distributeRewards(address[] calldata _recipients, uint256 _amount) external onlyOwner {
          for(uint i = 0; i < _recipients.length; i++) {
              // External call in a loop - potential for a DoS if one call fails
              (bool sent, ) = _recipients[i].call{value: _amount}("");
              require(sent, "Failed to send Ether"); // If one send fails, the entire operation reverts
          }
      }

      // Function to receive Ether, making this contract able to distribute rewards
      receive() external payable {}
  }

In the example above, the RewardDistributor contract represents a component of a system that can distribute ether among a set of recipients specified by the owner of the contract. The function distributeRewards() makes these distributions via calls inside of a loop, which could cause the function to fail if one of the calls failed, meaning no rewards would be distributed.

Technical example of how to fix the vulnerability

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

  contract RewardDistributorUpdated {
      address public owner;
      mapping(address => uint256) public rewards;

      constructor() {
          owner = msg.sender;
      }

      modifier onlyOwner() {
          require(msg.sender == owner, "Not the owner");
          _;
      }

      // Function for the owner to deposit rewards for a list of addresses
      function depositRewards(address[] calldata _recipients, uint256[] calldata _amounts) external payable onlyOwner {
          require(_recipients.length == _amounts.length, "Recipients and amounts mismatch");

          uint256 totalAmount = 0;
          for(uint i = 0; i < _recipients.length; i++) {
              rewards[_recipients[i]] += _amounts[i];
              totalAmount += _amounts[i];
          }

          require(msg.value >= totalAmount, "Insufficient value");
      }

      // Allows users to withdraw their rewards
      function withdrawRewards() external {
          uint256 reward = rewards[msg.sender];
          require(reward > 0, "No rewards to withdraw");

          // Reset the reward balance before transferring to prevent re-entrancy attacks
          rewards[msg.sender] = 0;

          (bool sent, ) = msg.sender.call{value: reward}("");
          require(sent, "Failed to send rewards");
      }

      // Function to receive Ether, allowing the contract to hold and distribute rewards
      receive() external payable {}

      // Function to allow the owner to withdraw contract balance (for example, to recover funds sent by mistake)
      function withdrawContractBalance() external onlyOwner {
          uint256 balance = address(this).balance;
          (bool sent, ) = owner.call{value: balance}("");
          require(sent, "Failed to send Ether");
      }
  }

In the revised example above, the RewardDistributorUpdated contract has been reworked to use an alternative design pattern for distribution of rewards, allowing users who have been allotted a reward to withdraw it on demand. Note that this pattern avoids attempting to make multiple calls to send rewards inside of a loop, such that one user's failure to receive their reward will not block other recipients from receiving theirs.