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

Off-by-one rarity thresholds mint 71% common / 25% rare / 4% legendary instead of 70 / 25 / 5

thedelph

Summary

The rarity roll is a value in [0, 99], but it is compared with <= against the cumulative thresholds COMMON_RARITY (70) and COMMON_RARITY + RARE_RARITY (95). Common therefore covers 71 values and legendary only 4, so legendary puppies are minted 20% less often than the contract advertises.

Description

src/PuppyRaffle.sol:139-146:

uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100; // 0..99
if (rarity <= COMMON_RARITY) { // 0..70 -> 71 values
tokenIdToRarity[tokenId] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) { // 71..95 -> 25 values
tokenIdToRarity[tokenId] = RARE_RARITY;
} else { // 96..99 -> 4 values
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}

The constants COMMON_RARITY = 70, RARE_RARITY = 25 and LEGENDARY_RARITY = 5 (:39, :44, :49) state a 70/25/5 split. The actual split is 71/25/4. For example, roll 95 is the 96th value; under 70/25/5 it should be legendary, but it is minted as rare.

Risk

Likelihood: High — every mint uses the thresholds.

Impact: Low

Winners get legendary NFTs less often than the documented odds (4% instead of 5%), which undervalues the prize distribution. There's no direct loss of funds, hence Low.

Proof of Concept

test/PuppyRaffleAudit.t.sol::test_PoC_RarityOffByOne. The test finds a caller whose roll is exactly 95 and shows that the minted token is RARE.

function test_PoC_RarityOffByOne() public {
address caller;
for (uint256 i = 1; i < 100000; i++) {
address c = address(uint160(0x10000 + i));
if (uint256(keccak256(abi.encodePacked(c, block.difficulty))) % 100 == 95) { caller = c; break; }
}
require(caller != address(0), "no caller found");
​
_enter(1, 4);
vm.warp(block.timestamp + duration + 1);
vm.prank(caller);
puppyRaffle.selectWinner();
assertEq(puppyRaffle.tokenIdToRarity(0), puppyRaffle.RARE_RARITY()); // should be LEGENDARY under 70/25/5
}

Run:

forge test --match-test test_PoC_RarityOffByOne -vvv

The output is in poc.txt.

Recommended Mitigation

Use strict < comparisons. rarity is uniform over 0–99, so this makes common 0–69 (70 values), rare 70–94 (25) and legendary 95–99 (5), matching the documented 70/25/5 odds.

- 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 {
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 42 minutes 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!