Array's length value is used directly in loop (removeRewardMine/MiningService.sol)

[code423n4]

Handle

ye0lde

Vulnerability details

Impact

Additional gas usage when an array's length value is used directly in a for loop.

Proof of Concept

The array's length value is used directly in a for loop here:
https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/MiningService.sol#L142-L145

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Change the function removeRewardMine to:

function removeRewardMine(address mine)
    public
    onlyRole(ADMIN_ROLE, "Must have admin privs")
  {
    if (!mineActive[mine]) {
      return;
    }
    mineActive[mine] = false;

    // Loop until the second last element
    uint256 lenMinus1 = mines.length - 1;
    for (uint i = 0; i < lenMinus1; i = i + 1) { 
      if (mines[i] == mine) {
        // Replace the current item with the last and pop the last away.
        mines[i] = mines[lenMinus1];
        mines.pop();
        return;
      }
    }

    // If we made it here then the mine being removed is the last item
    mines.pop();
  }

[0xScotch]

#106