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

Off-by-one in rarity thresholds: common 71%, legendary only 4% instead of 70/5

ward0x

Root + Impact

Description

  • The documented rarities are common 70%, rare 25%, legendary 5% (COMMON_RARITY = 70, RARE_RARITY = 25, LEGENDARY_RARITY = 5).

  • rarity is in [0, 99] and the checks use <=. Values 0..70 (71 values) are common, 71..95 (25 values) rare, and only 96..99 (4 values) legendary.

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

Distribution check: rarity takes 100 values 0..99.

Range Count Result Expected
0..70 71 common 70
71..95 25 rare 25
96..99 4 legendary 5

The PoC finds a caller whose rarity roll is exactly 70 and shows it is minted as common.

Risk

Likelihood:

  • Every mint.

Impact:

  • Legendary puppies are minted 20% less often than promised (4% instead of 5%) and common ones more often; NFT value and player expectations are off.

Proof of Concept

All tests run in a test contract that deploys PuppyRaffle(1e18, address(99), 1 days) and uses the helpers _players(start,n) (distinct addresses) and _enter(p) (pays 1e18 * p.length). Full file: test/AuditPoC.t.sol, forge test --mc AuditPoC passes.

function test_L2_rarityOffByOne() public {
address caller;
for (uint160 i = 1; i < 5000; i++) {
if (uint256(keccak256(abi.encodePacked(address(i), block.difficulty))) % 100 == 70) {
caller = address(i);
break;
}
}
assertTrue(caller != address(0));
_enter(_players(1, 4));
vm.warp(block.timestamp + 1 days);
vm.prank(caller);
r.selectWinner();
assertEq(r.tokenIdToRarity(0), r.COMMON_RARITY()); // 70 is the 71st common value
}

Recommended Mitigation

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

Lead Judging Commences

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