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

Unbounded Loop in `getUserMemorabiliaDetailed()` Leads to Permanent Denial of Service (DoS)

itsmeayaan

Root + Impact

Description

normal Behavior: the getUserMemorabiliaDetailed() function is supposed to return a detailed list of all memorabilia NFTs a specific user owns.

Issue: The function is completely non-functional. It contains incorrect loop logic and an unbounded loop structure that iterates through every NFT ever minted. This design guarantees that any call will fail by running out of gas once even a small number of items exist, making it a permanent Denial of Service vector.as the loop begins its search at ID 1 instead of the correct starting ID of 100, wasting gas on 99 useless iterations in every call.

  • Explain the specific issue or problem in one or more sentences

// Root cause in the codebase with @> marks to highlight the relevant section
uint256 public nextCollectionId = 100; //starting Id is 100
​
//in createMemorabiliaCollection() CollectionId is updated
uint256 collectionId = nextCollectionId++;
​
//in getUserMemorabiliaDetailed() 99 iterations are wasted
function getUserMemorabiliaDetailed(address user) external view returns (
uint256[] memory tokenIds,
uint256[] memory collectionIds,
uint256[] memory itemIds
) {
// First, count how many memorabilia they own
uint256 count = 0;
for (uint256 cId = 1; cId < nextCollectionId; cId++) { //here 99 calls are wasted causing Denial of service
for (uint256 iId = 1; iId < collections[cId].currentItemId; iId++) {
uint256 tokenId = encodeTokenId(cId, iId);
if (balanceOf(user, tokenId) > 0) {
count++;
}
}
}
//....
​

Risk

Likelihood: High

  • Reason : This will occur every time the function is called by a user or external service after a moderate number of memorabilia (e.g., a few hundred) have been minted across all collections.

Impact: High

  • Impact 1 :- Permanent Denial of Service, any off-chain application, such as the festival's website or a secondary marketplace, that relies on this function to display user assets will be permanently broken.

  • Impact 2:- Gas Wasting, any user or contract attempting to call this function will lose all gas provided for the transaction as it inevitably reverts, with no state change.

Proof of Concept

this test case show very high gas used in the checking 500 collectionId.

function testExploit_GetUserMemorabiliaDetailed_GasCost() public {
// 1. ARRANGE: Create a collection and mint 499 items to a user.
vm.startPrank(organizer);
uint256 collectionId = festivalPass.createMemorabiliaCollection("DoS Collection", "ipfs://dos", 1, 500, true);
vm.stopPrank();
​
vm.prank(address(festivalPass));
beatToken.mint(user1, 500);
​
vm.startPrank(user1);
for (uint i = 0; i < 499; i++) {
festivalPass.redeemMemorabilia(collectionId);
}
vm.stopPrank();
​
// 2. ACT: Measure the gas before and after the function call.
uint256 gasBefore = gasleft();
festivalPass.getUserMemorabiliaDetailed(user1);
uint256 gasAfter = gasleft();
​
uint256 gasUsed = gasBefore - gasAfter;
console.log("Gas used by getUserMemorabiliaDetailed() for 499 items:", gasUsed);
​
// 3. ASSERT: Assert the gas cost is excessively high.
// A cost over 10 million gas for a view function is a clear DoS vector.
assertGt(gasUsed, 10_00_000, "Gas cost is unacceptably high, proving DoS risk");
}

This test proves that the function wastes gas by looping 99 times unnecessarily before starting its actual work. It does this by measuring

function testExploit_WastedGasFromIncorrectLoopStart() public {
// This test is run right after setUp(), where no memorabilia has been created.
// The user owns no items, so the function should be very cheap.
​
// 1. ACT: Measure the gas cost of the function in its "empty" state.
uint256 gasBefore = gasleft();
festivalPass.getUserMemorabiliaDetailed(user1);
uint256 gasUsed = gasBefore - gasleft();
​
console.log("Gas cost for empty call (pure waste):", gasUsed);
​
// 2. ASSERT: Prove that the gas cost is significant even for an empty call.
// This cost is almost entirely due to the loop iterating from cId = 1 to 99.
// A simple view call should be much cheaper. A cost well above the base
// transaction fee proves significant waste from the useless iterations.
assertGt(gasUsed, 30_000, "Function consumes significant gas even when there is no work to do");
}

Recommended Mitigation

for more gas efficiency MerkleProof can be used.
for 99 wasted iteration below changes can be made.

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

Lead Judging Commences

inallhonesty Lead Judge 30 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

itsmeayaan Submitter
28 days ago
inallhonesty Lead Judge
28 days ago
inallhonesty Lead Judge 27 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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