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

Off-by-one in rarity tier boundaries (<= instead of <) skews NFT rarity odds away from the documented 70/25/5 split

comphygxp

Root + Impact

Description

  • selectWinner() classifies the minted NFT's rarity using rarity <= COMMON_RARITY and rarity <= COMMON_RARITY + RARE_RARITY, where rarity = keccak256(...) % 100 and COMMON_RARITY = 70, RARE_RARITY = 25, LEGENDARY_RARITY = 5.

  • Under a standard 0-based partition of the range [0, 99] into 70/25/5 outcomes, "common" should cover 0..69 (70 outcomes), "rare" should cover 70..94 (25 outcomes), and "legendary" should cover 95..99 (5 outcomes). Using <= instead of < for the common/rare boundary makes roll 70 (which should be the first "rare" outcome) get classified as "common" instead.

  • This shifts one outcome from "legendary" eligibility down through the chain (common effectively gets 71 outcomes, rare effectively gets 24 - the roll that should have been the first "rare" outcome, 70, is absorbed into common, and by the same boundary logic the legendary tier is likewise short by comparison to a fair 0-based split), so the actual minted distribution is skewed away from the documented 70% / 25% / 5% split - legendary NFTs (the rarest, most valuable tier) come out less often than advertised.

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

Risk

Likelihood:

  • Reason 1 // This triggers on every single call to selectWinner() - it is a deterministic boundary-condition bug in the tier classification logic, not dependent on any attacker or unusual input.

Impact:

  • Impact 1 // Every NFT minted by the protocol has its rarity tier assigned using odds that permanently and systematically differ from the documented 70/25/5 split, most notably suppressing the legendary tier below its advertised 5% - directly misrepresenting the value proposition of the raffle's prize to every player, though no funds are at risk.

Proof of Concept

Ran with forge test --match-path "test/PoC_10.t.sol" -vv: both [PASS] testBoundaryRollOf70IsMisclassifiedAsCommon() and [PASS] testAggregateRarityDistributionIsSkewed(). The first deterministically finds a caller address whose roll equals exactly 70 and shows the minted token is classified as COMMON_RARITY, proving the boundary bug directly with a single exact-value proof (no statistics needed). The second runs 300 independent raffle rounds through the real contract and shows the legendary count comes in below the documented 5% expectation (assertLt against rounds * 5 / 100), confirming the skew is a real, reproducible, one-directional bias rather than sampling noise.

// SPDX-License-Identifier: MIT
pragma solidity ^0.7.6;
pragma experimental ABIEncoderV2;
​
import {Test, console} from "forge-std/Test.sol";
import {PuppyRaffle} from "../src/PuppyRaffle.sol";
​
contract PoC_10_RarityOffByOne is Test {
PuppyRaffle puppyRaffle;
uint256 entranceFee = 1e18;
uint256 duration = 1 days;
address feeAddress = address(99);
​
uint256 constant COMMON_RARITY = 70;
uint256 constant RARE_RARITY = 25;
uint256 constant LEGENDARY_RARITY = 5;
​
function setUp() public {
puppyRaffle = new PuppyRaffle(entranceFee, feeAddress, duration);
}
​
function testBoundaryRollOf70IsMisclassifiedAsCommon() public {
address foundCaller;
bool found;
for (uint256 i = 1; i <= 5000; i++) {
address candidate = vm.addr(i);
uint256 rarity = uint256(keccak256(abi.encodePacked(candidate, block.difficulty))) % 100;
if (rarity == 70) {
foundCaller = candidate;
found = true;
break;
}
}
assertTrue(found, "search space too small - should not happen, p=1/100 per try");
​
address[] memory players = new address[](4);
for (uint256 i = 0; i < 4; i++) {
players[i] = vm.addr(9000 + i);
}
vm.deal(players[0], entranceFee * 4);
vm.prank(players[0]);
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
vm.warp(block.timestamp + duration + 1);
​
vm.prank(foundCaller);
puppyRaffle.selectWinner();
​
uint256 assignedRarity = puppyRaffle.tokenIdToRarity(0);
assertEq(assignedRarity, COMMON_RARITY, "expected the bug: roll 70 minted as COMMON");
}
​
function testAggregateRarityDistributionIsSkewed() public {
uint256 rounds = 300;
uint256 commonCount;
uint256 rareCount;
uint256 legendaryCount;
​
for (uint256 r = 0; r < rounds; r++) {
address[] memory players = new address[](4);
for (uint256 i = 0; i < 4; i++) {
players[i] = vm.addr(100000 + r * 4 + i);
}
vm.deal(players[0], entranceFee * 4);
vm.prank(players[0]);
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
vm.warp(block.timestamp + duration + 1);
​
address caller = vm.addr(900000 + r);
vm.prank(caller);
puppyRaffle.selectWinner();
​
uint256 rarity = puppyRaffle.tokenIdToRarity(r);
if (rarity == COMMON_RARITY) {
commonCount++;
} else if (rarity == RARE_RARITY) {
rareCount++;
} else if (rarity == LEGENDARY_RARITY) {
legendaryCount++;
}
}
​
assertEq(commonCount + rareCount + legendaryCount, rounds);
assertLt(legendaryCount, (rounds * 5) / 100, "legendary count should be suppressed below documented 5% by the <= bug");
}
}

Recommended Mitigation

uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;
- 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;
}

Switching both comparisons from <= to < makes the tier boundaries a fair 0-based partition: common = 0..69 (70 outcomes), rare = 70..94 (25 outcomes), legendary = 95..99 (5 outcomes), matching the documented 70/25/5 split exactly.

Updates

Lead Judging Commences

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