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

Off-By-One Error in Rarity Distribution Causes Incorrect Probabilities

dakthehack

Description

The PuppyRaffle::selectWinner function uses an incorrect rarity distribution algorithm. The code intends to distribute rarities as 70% Common, 25% Rare, and 5% Legendary, but due to an off-by-one error in the conditional logic, the actual distribution is 71% Common, 25% Rare, and only 4% Legendary.

Expected behavior: NFT rarities should be distributed exactly as documented: 70% Common, 25% Rare, 5% Legendary.

Actual behavior: The rarity distribution is skewed. Common NFTs appear 71% of the time (not 70%), and Legendary NFTs only appear 4% of the time (not 5%). This means users have a 20% lower chance of receiving a Legendary NFT than advertised.

uint256 public constant COMMON_RARITY = 70;
uint256 public constant RARE_RARITY = 25;
uint256 public constant LEGENDARY_RARITY = 5;
​
function selectWinner() external {
// ...
uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;
if (rarity <= COMMON_RARITY) { // @> 0-70 = 71 values (not 70!)
tokenIdToRarity[tokenId] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) { // @> 71-95 = 25 values (correct)
tokenIdToRarity[tokenId] = RARE_RARITY;
} else { // @> 96-99 = 4 values (not 5!)
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}
// ...
}

The issue is that rarity % 100 produces values from 0-99 (100 possible values), but the first condition uses <= which includes 0, creating 71 values for Common (0 through 70 inclusive) instead of 70. This leaves only 4 values (96-99) for Legendary instead of 5.

Risk

Likelihood:

  • High - Occurs on every winner selection

  • Affects 100% of NFT mints

  • Not random or probabilistic - always happens

Impact:

  • Violates documented rarity distribution

  • Users receive 20% fewer Legendary NFTs than expected (4% vs 5%)

  • If marketed as "5% legendary drop rate", this is false advertising

  • Unfair to users who participate expecting correct odds

  • Could lead to reputation damage if discovered post-launch

Proof of Concept

Add to test/PuppyRaffleTest.t.sol:

/**
* @notice Tests the actual distribution of rarity values
* @dev Simulates rarity calculation for all 100 possible values (0-99) to check distribution
*/
function test_rarityDistribution() public {
emit log("=== Testing Rarity Distribution ===");
emit log("Expected: 70% Common, 25% Rare, 5% Legendary");
emit log("");
// Test all possible rarity values (0-99)
uint256 commonCount = 0;
uint256 rareCount = 0;
uint256 legendaryCount = 0;
for (uint256 rarity = 0; rarity < 100; rarity++) {
if (rarity <= puppyRaffle.COMMON_RARITY()) { // <= 70
commonCount++;
} else if (rarity <= puppyRaffle.COMMON_RARITY() + puppyRaffle.RARE_RARITY()) { // <= 95
rareCount++;
} else { // 96-99
legendaryCount++;
}
}
emit log("=== Actual Distribution ===");
emit log_named_uint("Common count (0-70)", commonCount);
emit log_named_uint("Rare count (71-95)", rareCount);
emit log_named_uint("Legendary count (96-99)", legendaryCount);
emit log("");
emit log_named_uint("Common %", commonCount);
emit log_named_uint("Rare %", rareCount);
emit log_named_uint("Legendary %", legendaryCount);
emit log("");
emit log("=== Problem Found ===");
emit log("Legendary should be 5% but is only 4%!");
emit log("Common is 71% instead of 70%");
// Assertions
assertEq(commonCount, 71, "Common should be 71 values (0-70 inclusive)");
assertEq(rareCount, 25, "Rare should be 25 values (71-95)");
assertEq(legendaryCount, 4, "Legendary is only 4 values (96-99), not 5!");
}

Run: forge test --match-test test_rarityDistribution -vv --offline

Output:

=== Testing Rarity Distribution ===
Expected: 70% Common, 25% Rare, 5% Legendary
​
=== Actual Distribution ===
Common count (0-70): 71
Rare count (71-95): 25
Legendary count (96-99): 4
​
Common %: 71
Rare %: 25
Legendary %: 4
​
=== Problem Found ===
Legendary should be 5% but is only 4%!
Common is 71% instead of 70%

The test proves that across all 100 possible random values (0-99), only 4 result in Legendary (96, 97, 98, 99), not 5 as intended.

Recommended Mitigation

Change the first condition from <= to < to ensure exactly 70 values for Common:

function selectWinner() external {
// ...
uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;
- if (rarity <= COMMON_RARITY) {
+ if (rarity < COMMON_RARITY) { // 0-69 = 70 values
tokenIdToRarity[tokenId] = COMMON_RARITY;
- } else if (rarity <= COMMON_RARITY + RARE_RARITY) {
+ } else if (rarity < COMMON_RARITY + RARE_RARITY) { // 70-94 = 25 values
tokenIdToRarity[tokenId] = RARE_RARITY;
} else { // 95-99 = 5 values
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}
// ...
}

This gives the correct distribution: 70% Common (0-69), 25% Rare (70-94), 5% Legendary (95-99).

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 8 days 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!