Beatland Festival
AI First Flight #4
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Incorrect collection ID iteration in `getUserMemorabiliaDetailed()` wastes gas and iterates through non-existent collections

0xstn_

[M-5] Incorrect collection ID iteration in getUserMemorabiliaDetailed() wastes gas and iterates through non-existent collections

Description

The getUserMemorabiliaDetailed() function iterates through collection IDs starting from 1, but memorabilia collections actually start at ID 100 (nextCollectionId = 100). Collection IDs 1-3 are reserved for pass types (GENERAL_PASS, VIP_PASS, BACKSTAGE_PASS), and IDs 4-99 do not exist as collections.

function getUserMemorabiliaDetailed(address user) external view returns (...) {
uint256 count = 0;
for (uint256 cId = 1; cId < nextCollectionId; cId++) { // @audit Starts at 1 instead of 100
for (uint256 iId = 1; iId < collections[cId].currentItemId; iId++) {
uint256 tokenId = encodeTokenId(cId, iId);
if (balanceOf(user, tokenId) > 0) {
count++;
}
}
}
// ... second loop has same issue
}

This causes the function to unnecessarily iterate through 99 non-memorabilia collection IDs before reaching the actual memorabilia starting at ID 100.

Impact

  • Gas Waste: The function iterates through 99 non-existent collection IDs (1-99) on every call, performing unnecessary storage reads and operations

  • Inefficiency: For each of the 99 iterations, the function reads collections[cId].currentItemId which returns 0 for non-existent collections, skipping the inner loop but still consuming gas

  • Incorrect Logic: The function conflates pass IDs (1-3) with memorabilia collection IDs (≥100), violating the intended separation

While this is primarily a gas efficiency issue, it demonstrates a fundamental misunderstanding of the collection ID structure in the codebase.

Proof of Concept

// Contract initialization
uint256 public nextCollectionId = 100; // Memorabilia starts at 100
​
// getUserMemorabiliaDetailed() iterates from 1
for (uint256 cId = 1; cId < nextCollectionId; cId++) { // Will iterate: 1,2,3,4...99,100,101...
// For cId 1-3: These are passes, not memorabilia collections
// For cId 4-99: These collections don't exist (currentItemId = 0)
// For cId ≥100: These are actual memorabilia collections
}

Gas waste calculation:

  • If nextCollectionId = 105 (5 memorabilia collections created)

  • Function iterates 104 times (1 to 104)

  • Only 5 iterations (100-104) are meaningful

  • 99 iterations (1-99) are wasted

Mitigation

Start the iteration from 100 to skip non-memorabilia IDs:

function getUserMemorabiliaDetailed(address user)
external
view
returns (uint256[] memory tokenIds, uint256[] memory collectionIds, uint256[] memory itemIds)
{
uint256 count = 0;
- for (uint256 cId = 1; cId < nextCollectionId; cId++) {
+ for (uint256 cId = 100; cId < nextCollectionId; cId++) {
for (uint256 iId = 1; iId < collections[cId].currentItemId; iId++) {
uint256 tokenId = encodeTokenId(cId, iId);
if (balanceOf(user, tokenId) > 0) {
count++;
}
}
}
​
tokenIds = new uint256[](count);
collectionIds = new uint256[](count);
itemIds = new uint256[](count);
​
uint256 index = 0;
- for (uint256 cId = 1; cId < nextCollectionId; cId++) {
+ for (uint256 cId = 100; cId < nextCollectionId; cId++) {
for (uint256 iId = 1; iId < collections[cId].currentItemId; iId++) {
uint256 tokenId = encodeTokenId(cId, iId);
if (balanceOf(user, tokenId) > 0) {
tokenIds[index] = tokenId;
collectionIds[index] = cId;
itemIds[index] = iId;
index++;
}
}
}
​
return (tokenIds, collectionIds, itemIds);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!