MyCut

First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Invalid

Mismatched Array Lengths in Constructor Could Lead to Potential Reverts or Incomplete Initialization

Summary

The constructor of the contract Pot.sol fails to enforce a length check between the i_players and i_rewards arrays. This creates a vulnerability where an out-of-bounds error may occur if the lengths of these arrays are mismatched, leading to potential transaction reverts or incomplete initialization of the contract’s state.

Vulnerability Details

In the constructor, i_players and i_rewards are passed as parameters and then used in a loop to initialize the playersToRewards mapping. However, the contract does not check whether the lengths of these two arrays are equal. If the arrays have different lengths, the loop may attempt to access an index in the shorter array that doesn't exist, which will cause the contract to revert due to an out-of-bounds error. Alternatively, if the players' array is shorter, some rewards might not be assigned, leaving part of the playersToRewards mapping uninitialized.

This lack of a length check can lead to unintended behavior and transaction failures during contract deployment.

Impact

  • Out-of-Bounds Revert: If i_players is longer than i_rewards, the contract will attempt to access an index of i_rewards that doesn't exist, causing a revert and preventing the contract from being deployed.

  • Incomplete State Initialization: If i_rewards is longer than i_players, the rewards may not be assigned to all players, resulting in incomplete initialization of the playersToRewards mapping.

  • Incorrect Data Logic: Even if no immediate revert occurs, mismatched lengths could result in incorrect reward distribution, causing incorrect associations between players and rewards.

POC

Here’s a simplified example showing the vulnerability:

contract ConstructorTest {
address[] public i_players;
uint256[] public i_rewards;
mapping(address => uint256) public playersToRewards;
constructor(address[] memory players, uint256[] memory rewards) {
// No length check
i_players = players;
i_rewards = rewards;
for (uint256 i = 0; i < i_players.length; i++) {
// Vulnerable to out-of-bounds error if players and rewards arrays are of different lengths
playersToRewards[i_players[i]] = i_rewards[i];
}
}
}

In a scenario where:

  • i_players.length = 3

  • i_rewards.length = 2
    The loop will try to access i_rewards[2], which doesn't exist, causing a revert and failing the contract deployment.

Tools Used

Manual Analysis

Recommendations

To prevent this vulnerability, always check that the lengths of the i_players and i_rewards arrays are equal before proceeding with the loop.

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
+ require(players.length == rewards.length, "Mismatched players and rewards");
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
i_deployedAt = block.timestamp;
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}
Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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