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

Inclusive rarity boundaries produce 71/25/4 instead of 70/25/5

rvoffindings

Root + Impact

Description

The rarity roll is reduced to the integer domain 0..99, but the contract compares it against inclusive upper bounds. As a result, common receives 0..70 (71 values), rare receives 71..95 (25 values), and legendary receives only 96..99 (4 values), instead of the declared 70/25/5 distribution.

uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;
// @> Includes both 0 and 70: 71 outcomes.
if (rarity <= COMMON_RARITY) {
tokenIdToRarity[tokenId] = COMMON_RARITY;
// @> Includes 71 through 95: the legendary range loses value 95.
} else if (rarity <= COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[tokenId] = RARE_RARITY;
} else {
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}

The persisted value is later used by tokenURI() to select the NFT name and image, so the boundary error affects collection metadata and scarcity rather than only an intermediate variable.

Risk

Likelihood: High

Every successful mint executes this classifier. No special privilege or attacker-controlled state is required for the incorrect boundaries to be reachable.

Impact: Low

The collection mints common, rare, and legendary outcomes in a 71/25/4 domain split instead of the advertised 70/25/5. This affects rarity integrity and expected scarcity, but it does not independently steal funds, block settlement, or grant privileges.

Proof of Concept

Because % 100 creates a finite domain, complete enumeration is sufficient:

Branch Current interval Count
Common 0..70 71
Rare 71..95 25
Legendary 96..99 4

The boundary values demonstrate the mismatch directly: 70 is classified as common although it should begin rare, and 95 is classified as rare although it should begin legendary.

uint256 common;
uint256 rare;
uint256 legendary;
for (uint256 roll = 0; roll < 100; roll++) {
if (roll <= 70) common++;
else if (roll <= 95) rare++;
else legendary++;
}
assertEq(common, 71);
assertEq(rare, 25);
assertEq(legendary, 4);

This was confirmed by static enumeration of challenge commit 08e5b1fc6939b8da7792b2d13e43000c519d8897. The aggregate 24-test log did not contain a rarity-boundary test, so no EVM execution is claimed for this specific finding.

Recommended Mitigation

Use exclusive upper bounds and isolate the decision in a small pure helper.

- 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;
}

Add boundary tests for 69, 70, 94, and 95, plus a loop over all 100 residues asserting exactly 70 common, 25 rare, 5 legendary, and a total of 100.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 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!