First Flight #20: The Predicter
First Flight #20
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unbounded Iteration Vulnerability

ghufranhassan1

Summary

https://github.com/Cyfrin/2024-07-the-predicter/blob/main/src/ThePredicter.sol#L121

The withdraw() function iterates over the players array without any gas consumption safeguards. As the length of the players array increases, the gas required to execute the loop also increases linearly. This can lead to transactions failing due to running out of gas.

for (uint256 i = 0; i < players.length; ++i) {
int8 cScore = scoreBoard.getPlayerScore(players[i]);
if (cScore > maxScore) maxScore = cScore;
if (cScore > 0) totalPositivePoints += cScore;
}

Vulnerability Details

The withdraw() function iterates over the players array using a for loop. If the number of players in the players array is excessively large, this loop can consume a significant amount of gas, potentially exceeding the block gas limit and causing the transaction to fail.

Impact

Transactions attempting to call the withdraw() function may fail if the gas required exceeds the block gas limit. The following POC in foundry demonstrates the exploitation of vulnerabily. This test simulates registering a large number of players to demonstrate how the withdraw function may run out of gas due to the unbounded iteration over the players array. The vm.expectRevert() is used to assert that the transaction will revert, which indicates the potential gas issue caused by the large loop iteration.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import "forge-std/Test.sol";
import "../src/ThePredicter.sol";
import "../src/ScoreBoard.sol";
​
contract ThePredicterTest is Test {
ThePredicter public thePredicter;
ScoreBoard public scoreBoard;
address public organizer = address(0x1);
address public player = address(0x2);
uint256 public constant ENTRANCE_FEE = 1 ether;
uint256 public constant PREDICTION_FEE = 0.1 ether;
​
function setUp() public {
scoreBoard = new ScoreBoard();
thePredicter = new ThePredicter(address(scoreBoard), ENTRANCE_FEE, PREDICTION_FEE);
vm.deal(player, 10 ether);
}
​
function testWithdrawWithLargePlayerArray() public {
// Add many players to simulate the issue
for (uint256 i = 0; i < 300; i++) {
address newPlayer = address(uint160(i + 1));
vm.deal(newPlayer, 10 ether);
vm.prank(newPlayer);
thePredicter.register{value: ENTRANCE_FEE}();
vm.prank(organizer);
thePredicter.approvePlayer(newPlayer);
}
​
vm.prank(player);
vm.expectRevert(); // Expecting the transaction to revert due to out-of-gas
thePredicter.withdraw();
}
}
​

Tools Used

Manual Review and Foundry

Recommendations

Impose a maximum limit on the number of players to keep the gas consumption manageable.

uint256 public constant MAX_PLAYERS = 100;
​
function approvePlayer(address player) public {
if (msg.sender != organizer) {
revert ThePredicter__UnauthorizedAccess();
}
if (players.length >= MAX_PLAYERS) {
revert ThePredicter__AllPlacesAreTaken();
}
if (playersStatus[player] == Status.Pending) {
playersStatus[player] = Status.Approved;
players.push(player);
}
}
​
Updates

Lead Judging Commences

NightHawK Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.