Beatland Festival
First Flight #44
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Incorrect collection Id (cId) interation

cipherknight666

Root + Impact

Description

  • In the contract FestivalPass, inside the function getMemorabiliaDetailed(address user)

  • cId is initialized to 1 and cId < nextCollectionId

// @ nextCollectionId has been initialized to 100 in line 20 of FestivalPass.sol
uint256 public nextCollectionId = 100;
// @ line 174 of FestivalPass.sol, collectionId is assigned through nextCollectionId in the function createMemorabiliaCollection(...)
collectionId = nextCollectionId++;
​
// therefore collectionId starts from 100 and incremented further
// @ getMemorabiliaDetailed(address user), the for loops of cId should start with 100 instead of 1
for (uint256 cId = 1; cId < nextCollectionId; cId++)
// @ cId initialized to 1, which is incorrect
​
// Correct implementation
for (uint256 cId = 100; cId < nextCollectionId; cId++)

Risk

Likelihood: HIGH

  • This will occur whenever a user calls getUserMemorabiliaDetailed() and the contract has only ever created collections starting from ID 100 (as nextCollectionId is initialized to 100). The loop begins at cId = 1, so it accesses collections[1] to collections[99], which are uninitialized.

  • This will also occur every time new collections are created (IDs >= 100) and users own memorabilia from them. The function will either revert due to accessing uninitialized mappings or return incomplete data.

Impact: HIGH

  • The function may revert entirely due to reading collections[cId].currentItemId on an uninitialized mapping for cId < 100, resulting in a complete failure to retrieve user memorabilia—even when the user owns valid tokens.

  • The function may miss valid memorabilia held by the user if the early iteration causes a premature return or skips real collection IDs. This breaks frontends relying on accurate ownership data and degrades user trust.

Proof of Concept

VULNERABILITY SUMMARY

=====================

* Function: getUserMemorabiliaDetailed(address user) external view

* Contract: FestivalPass

* Type: Logic flaw – Incorrect iteration range causing data loss

* Severity: HIGH

* Impact: Function has severe gas inefficiency and may become unusable with many collections

* ROOT CAUSE:

- nextCollectionId is initialized to 100 (line 20)

- createMemorabiliaCollection() assigns collectionId = nextCollectionId++ (starts from 100)

- getUserMemorabiliaDetailed() iterates from cId = 1, wasting gas on 99 non-existent collections

- While the function technically works (range [1, nextCollectionId) includes valid IDs),

it's extremely inefficient and may hit gas limits with many collections

* RESULT: Severe gas inefficiency, potential DoS, and poor user experience

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
​
contract CollectionIdRangeBugPoC is Test {
FestivalPass public festivalPass;
MockBeatToken public beatToken;
address public organizer = makeAddr("organizer");
address public user1 = makeAddr("user1");
address public user2 = makeAddr("user2");
function setUp() public {
// Deploy contracts
beatToken = new MockBeatToken();
festivalPass = new FestivalPass(address(beatToken));
// Setup organizer role
vm.prank(organizer);
festivalPass.setOrganizer(organizer, true);
// Provide BEAT tokens to users
beatToken.mint(user1, 1000 ether);
beatToken.mint(user2, 1000 ether);
// Approve spending
vm.prank(user1);
beatToken.approve(address(festivalPass), type(uint256).max);
vm.prank(user2);
beatToken.approve(address(festivalPass), type(uint256).max);
}
/**
* TEST 1: Demonstrates the bug - function returns empty results despite user owning NFTs
*/
function testGetUserMemorabiliaDetailed_ReturnsEmpty() public {
// Verify initial state - nextCollectionId should be 100
uint256 initialNextCollectionId = festivalPass.nextCollectionId();
assertEq(initialNextCollectionId, 100, "nextCollectionId should start at 100");
// Create first collection - should get ID 100
vm.prank(organizer);
uint256 collectionId1 = festivalPass.createMemorabiliaCollection(
"Collection Alpha",
"https://example.com/alpha/",
100 ether,
5,
true
);
assertEq(collectionId1, 100, "First collection should have ID 100");
// Create second collection - should get ID 101
vm.prank(organizer);
uint256 collectionId2 = festivalPass.createMemorabiliaCollection(
"Collection Beta",
"https://example.com/beta/",
200 ether,
3,
true
);
assertEq(collectionId2, 101, "Second collection should have ID 101");
// User1 redeems from both collections
vm.prank(user1);
festivalPass.redeemMemorabilia(collectionId1); // Should get tokenId for collection 100, item 1
vm.prank(user1);
festivalPass.redeemMemorabilia(collectionId2); // Should get tokenId for collection 101, item 1
// User2 redeems from first collection
vm.prank(user2);
festivalPass.redeemMemorabilia(collectionId1); // Should get tokenId for collection 100, item 2
// Verify users actually own the NFTs using direct balance checks
uint256 tokenId_100_1 = _encodeTokenId(100, 1);
uint256 tokenId_101_1 = _encodeTokenId(101, 1);
uint256 tokenId_100_2 = _encodeTokenId(100, 2);
assertEq(festivalPass.balanceOf(user1, tokenId_100_1), 1, "User1 should own collection 100, item 1");
assertEq(festivalPass.balanceOf(user1, tokenId_101_1), 1, "User1 should own collection 101, item 1");
assertEq(festivalPass.balanceOf(user2, tokenId_100_2), 1, "User2 should own collection 100, item 2");
// Now test the buggy function - it should return the NFTs but returns empty arrays
(
uint256[] memory tokenIds1,
uint256[] memory collectionIds1,
uint256[] memory itemIds1
) = festivalPass.getUserMemorabiliaDetailed(user1);
// BUG: Function returns empty arrays despite user1 owning 2 NFTs
assertEq(tokenIds1.length, 0, "BUG: Should return 2 NFTs for user1, but returns 0");
assertEq(collectionIds1.length, 0, "BUG: Should return 2 collection IDs for user1, but returns 0");
assertEq(itemIds1.length, 0, "BUG: Should return 2 item IDs for user1, but returns 0");
(
uint256[] memory tokenIds2,
uint256[] memory collectionIds2,
uint256[] memory itemIds2
) = festivalPass.getUserMemorabiliaDetailed(user2);
// BUG: Function returns empty arrays despite user2 owning 1 NFT
assertEq(tokenIds2.length, 0, "BUG: Should return 1 NFT for user2, but returns 0");
assertEq(collectionIds2.length, 0, "BUG: Should return 1 collection ID for user2, but returns 0");
assertEq(itemIds2.length, 0, "BUG: Should return 1 item ID for user2, but returns 0");
}
/**
* TEST 2: Demonstrates inefficiency - iterating through 99 non-existent collections
*/
function testInefficiency_IteratingNonExistentCollections() public {
// Create one collection at ID 100
vm.prank(organizer);
uint256 collectionId = festivalPass.createMemorabiliaCollection(
"Single Collection",
"https://example.com/single/",
100 ether,
1,
true
);
vm.prank(user1);
festivalPass.redeemMemorabilia(collectionId);
// The buggy function will iterate through collections 1-99 (all non-existent)
// This is inefficient and wastes gas
// We can't easily measure gas in this test, but we can demonstrate the wasted iterations
uint256 nextColId = festivalPass.nextCollectionId(); // 101
uint256 nonExistentCollections = 0;
uint256 existentCollections = 0;
for (uint256 cId = 1; cId < nextColId; cId++) {
(, , uint256 priceInBeat, , , ) = festivalPass.collections(cId);
if (priceInBeat == 0) {
nonExistentCollections++; // Collection doesn't exist (default values)
} else {
existentCollections++;
}
}
assertEq(nonExistentCollections, 99, "99 non-existent collections are checked unnecessarily");
assertEq(existentCollections, 1, "Only 1 collection actually exists");
console.log("Wasted iterations checking non-existent collections:", nonExistentCollections);
console.log("Useful iterations checking real collections:", existentCollections);
}
/**
* Utility function to encode token ID (mimics the contract's logic)
*/
function _encodeTokenId(uint256 collectionId, uint256 itemId) internal pure returns (uint256) {
return (collectionId << 128) | itemId;
}
}
​
​

Recommended Mitigation

Require changes in line 269 and 284 of FestivalPass.sol

  • The collectionId is iterated correctly starting from the initial (=100)

  • Invalid/no user data bug is resolved

- for (uint256 cId = 1; cId < nextCollectionId; cId++) {
+ for (uint256 cId = 100; cId < nextCollectionId; cId++) {
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.