Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Off-by-one error in PuppyRaffle::selectWinner() rarity threshold reduces legendary NFT probability by 20%

georgibuilds

Root + Impact

Description

  • PuppyRaffle::selectWinner() uses <= COMMON_RARITY (70) to
    classify NFT rarity. Since rarity is computed as 0-99
    (100 possible outcomes), <= 70 captures 71 outcomes instead
    of the documented 70. This permanently skews the distribution:
    Common receives 71% instead of 70%, and Legendary receives
    4% instead of 5% — a 20% reduction in legendary NFT probability.

uint256 rarity = uint256(keccak256(
abi.encodePacked(msg.sender, block.difficulty)
)) % 100;
// rarity = 0 to 99 (100 possible outcomes)
​
// @> <= captures 71 outcomes (0-70), not 70 (0-69)
if (rarity <= COMMON_RARITY) {
tokenIdToRarity[tokenId] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[tokenId] = RARE_RARITY;
} else {
// @> Only 4 outcomes remain (96-99) instead of 5 (95-99)
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}

Risk

Likelihood:

  • Affects every single raffle round permanently

  • No special conditions required

  • Built into the contract logic from deployment

Impact:

  • Legendary NFT probability reduced from 5% to 4%

  • Common NFT probability inflated from 70% to 71%

  • Players misled about actual drop rates

  • Economic value of legendary NFTs artificially inflated
    due to reduced supply

Proof of Concept

Attack Path:

  1. Contract deploys with COMMON_RARITY = 70

  2. rarity computed as 0-99 (100 outcomes)

  3. if (rarity <= 70) captures outcomes 0,1,2...70 = 71 outcomes

  4. Legendary only gets outcomes 96,97,98,99 = 4 outcomes

  5. Every raffle round: Common +1%, Legendary -1%

function test_rarity_offbyone() public {
uint256 commonOutcomes = 0;
uint256 rareOutcomes = 0;
uint256 legendaryOutcomes = 0;
​
for (uint256 r = 0; r < 100; r++) {
if (r <= puppyRaffle.COMMON_RARITY()) {
commonOutcomes++;
} else if (r <= puppyRaffle.COMMON_RARITY() +
puppyRaffle.RARE_RARITY()) {
rareOutcomes++;
} else {
legendaryOutcomes++;
}
}
​
console.log("Common outcomes (should be 70):", commonOutcomes);
console.log("Rare outcomes (should be 25):", rareOutcomes);
console.log("Legendary outcomes (should be 5):", legendaryOutcomes);
​
assertEq(commonOutcomes, 71); // BUG: +1
assertEq(rareOutcomes, 25); // correct
assertEq(legendaryOutcomes, 4); // BUG: -1
}

Recommended Mitigation

Change <= to < for the Common rarity check. This correctly
captures outcomes 0-69 (70 outcomes = 70%) for Common,
leaving outcome 70 in the Rare bucket and restoring
Legendary to 5 outcomes (95-99 = 5%).

- if (rarity <= COMMON_RARITY) {
+ if (rarity < COMMON_RARITY) {
tokenIdToRarity[tokenId] = COMMON_RARITY;
- } else if (rarity <= COMMON_RARITY + RARE_RARITY) {
+ } else if (rarity < COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[tokenId] = RARE_RARITY;
} else {
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[L-03] Participants are mislead by the rarity chances.

## Description The drop chances defined in the state variables section for the COMMON and LEGENDARY are misleading. ## Vulnerability Details The 3 rarity scores are defined as follows: ``` uint256 public constant COMMON_RARITY = 70; uint256 public constant RARE_RARITY = 25; uint256 public constant LEGENDARY_RARITY = 5; ``` This implies that out of a really big number of NFT's, 70% should be of common rarity, 25% should be of rare rarity and the last 5% should be legendary. The `selectWinners` function doesn't implement these numbers. ``` uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100; if (rarity <= COMMON_RARITY) { tokenIdToRarity[tokenId] = COMMON_RARITY; } else if (rarity <= COMMON_RARITY + RARE_RARITY) { tokenIdToRarity[tokenId] = RARE_RARITY; } else { tokenIdToRarity[tokenId] = LEGENDARY_RARITY; } ``` The `rarity` variable in the code above has a possible range of values within [0;99] (inclusive) This means that `rarity <= COMMON_RARITY` condition will apply for the interval [0:70], the `rarity <= COMMON_RARITY + RARE_RARITY` condition will apply for the [71:95] rarity and the rest of the interval [96:99] will be of `LEGENDARY_RARITY` The [0:70] interval contains 71 numbers `(70 - 0 + 1)` The [71:95] interval contains 25 numbers `(95 - 71 + 1)` The [96:99] interval contains 4 numbers `(99 - 96 + 1)` This means there is a 71% chance someone draws a COMMON NFT, 25% for a RARE NFT and 4% for a LEGENDARY NFT. ## Impact Depending on the info presented, the raffle participants might be lied with respect to the chances they have to draw a legendary NFT. ## Recommendations Drop the `=` sign from both conditions: ```diff -- if (rarity <= COMMON_RARITY) { ++ if (rarity < COMMON_RARITY) { tokenIdToRarity[tokenId] = COMMON_RARITY; -- } else if (rarity <= COMMON_RARITY + RARE_RARITY) { ++ } else if (rarity < COMMON_RARITY + RARE_RARITY) { tokenIdToRarity[tokenId] = RARE_RARITY; } else { tokenIdToRarity[tokenId] = LEGENDARY_RARITY; } ```

Support

FAQs

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

Give us feedback!