Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: medium
Invalid

State is written to storage before the duplicate check in `PuppyRaffle::enterRaffle`, wasting gas on reverted transactions

smilenot

[G-3] State is written to storage before the duplicate check in PuppyRaffle::enterRaffle, wasting gas on reverted transactions

Description: PuppyRaffle::enterRaffle pushes every address in newPlayers into the players storage array before verifying that no duplicates exist. If a duplicate is found, the transaction reverts and all state changes are rolled back, but the gas spent on those writes is not refunded to the caller. Each players.push() on a fresh storage slot costs gas, so a call carrying a duplicate near the end of a large array pays for every preceding write before the check ever runs.

function enterRaffle(address[] memory newPlayers) public payable {
require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");
​
@> for (uint256 i = 0; i < newPlayers.length; i++) {
@> players.push(newPlayers[i]);
}
​
// Check for duplicates
for (uint256 i = 0; i < players.length - 1; i++) {
for (uint256 j = i + 1; j < players.length; j++) {
require(players[i] != players[j], "PuppyRaffle: Duplicate player");
}
}
emit RaffleEnter(newPlayers);
}

Impact: Callers submitting an array containing a duplicate pay for storage writes that are discarded. With 50 new players and a duplicate in the last position, unnecessary gas is consumed before the revert. The contract itself is not harmed, but the cost falls entirely on the user.

Recommended Mitigation: Validate before writing, so that a rejected call reverts before any storage is touched.

function enterRaffle(address[] memory newPlayers) public payable {
require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");
​
- for (uint256 i = 0; i < newPlayers.length; i++) {
- players.push(newPlayers[i]);
- }
-
- // Check for duplicates
- for (uint256 i = 0; i < players.length - 1; i++) {
- for (uint256 j = i + 1; j < players.length; j++) {
- require(players[i] != players[j], "PuppyRaffle: Duplicate player");
- }
- }
+ for (uint256 i = 0; i < newPlayers.length; i++) {
+ for (uint256 j = 0; j < players.length; j++) {
+ require(players[j] != newPlayers[i], "PuppyRaffle: Duplicate player");
+ }
+ for (uint256 k = i + 1; k < newPlayers.length; k++) {
+ require(newPlayers[i] != newPlayers[k], "PuppyRaffle: Duplicate player");
+ }
+ }
+
+ for (uint256 i = 0; i < newPlayers.length; i++) {
+ players.push(newPlayers[i]);
+ }
emit RaffleEnter(newPlayers);
}

Note: the rewritten check covers both cases the original relied on: duplicates between the new entries and the existing players, and duplicates within newPlayers itself.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!