MyCut
First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Missing check length for array of players and array of rewards causes token rewards stack in protocol or logic error

hrmneffdii

Summary

In constructor of Pot.sol, there is no checking length for array of players and array of rewards can lead either revert array out of bound or last array of rewards not used.

Vulnerability Details

In constructor of Pot.sol, there is following code below :

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
i_deployedAt = block.timestamp;
​
// i_token.transfer(address(this), i_totalRewards);
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}

There is no checking length for array of players and array of rewards. Sometimes, maybe userhas mistake when insert a value of players or value of rewards.

For example :

// mistake type 1 can lead a reward still on protocol
// length of players more shorter than rewards
​
address[] players = [address("p1"), address("p2")];
uint256[] rewards = [10, 30, 40];
​
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
// players[0] with rewards[0]
// players[1] with rewards[1]
​
// how about rewards[2] ?? it will stay on protocol forever ??
}
​
// mistake type 2 can lead revert of array out of bound
// length of players more longer than rewards
address[] players = [address("p1"), address("p2")];
uint256[] rewards = [10];
​
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
// players[0] with rewards[0]
// players[1] with ??
​
// it will revert because rewards not have second index
}
​

Impact

The protocol will either revert because of array out of bound or rewards will stay on protocol forever.

Tools Used

Manual Review ~ Foundry

Recommendations

Add checking for length of players and rewards to ensure they are exactly same :

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
i_deployedAt = block.timestamp;
​
// i_token.transfer(address(this), i_totalRewards);
+ require(players.length == rewards.length, "Players and Rewards must at the same length");
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}
Updates

Lead Judging Commences

equious Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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