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

[H-01]: Unbounded Loop in `getUserMemorabiliaDetailed` Will Inevitably Lead to Denial of Service

mddragon18

[H-01]: Unbounded Loop in getUserMemorabiliaDetailed Will Inevitably Lead to Denial of Service

Description

  • The function getUserMemorabiliaDetailed is designed to return all memorabilia NFTs owned by a user. It does this by iterating through every possible collectionId and itemId to check the user's balance for each potential tokenId.

  • As the number of collections and minted memorabilia increases, the gas cost required to execute these nested loops will grow without limit.

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++) {
for (uint256 iId = 1; iId < collections[cId].currentItemId; iId++) {
uint256 tokenId = encodeTokenId(cId, iId);
if (balanceOf(user, tokenId) > 0) {
count++;
}
}
}
// Then populate arrays
tokenIds = new uint256[](count);
collectionIds = new uint256[](count);
itemIds = new uint256[](count);
uint256 index = 0;
@> for (uint256 cId = 1; 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);
}

Risk

Likelihood:

  • When the gas cost will exceed the block gas limit, causing the transaction to revert every time it is called.

Impact:

  • Eventually, the gas cost will exceed the block gas limit, causing the transaction to revert every time it is called. At this point, the function will become permanently unusable for all users, including those with only a few memorabilia, effectively creating a permanent Denial of Service (DoS) for a core feature of the contract.

Proof of Concept

(Conceptual) The organizer creates 100 memorabilia collections via createMemorabiliaCollection. nextCollectionId is now 200.

  • Users redeem 500 items from each of these 100 collections. collections[cId].currentItemId is now ~501 for each collection.

  • A user who owns several memorabilia calls getUserMemorabiliaDetailed.

  • The outer loop runs from cId = 1 to 199. The inner loop runs from iId = 1 to ~500.

function test_DoS_GetUserMemorabiliaDetailed() public {
uint256 NUM_COLLECTIONS = 40;
uint256 ITEMS_PER_COLLECTION = 100;
​
vm.prank(address(festivalPass));
beatToken.mint(organizer, 1_000_000_000_000_000_000e18);
​
vm.startPrank(organizer);
for (uint256 i = 0; i < NUM_COLLECTIONS; i++) {
// Create a new collection. The first collectionId created will be 100.
uint256 collectionId = festivalPass.createMemorabiliaCollection(
"DoS Collection",
"ipfs://dos/",
1e18,
ITEMS_PER_COLLECTION + 1,
true
);
​
for (uint256 j = 0; j < ITEMS_PER_COLLECTION; j++) {
// It doesn't matter who mints; the function iterates through all possible
// items regardless of ownership. Here, the organizer mints to themself.
festivalPass.redeemMemorabilia(collectionId);
}
}
vm.stopPrank();
uint256 gasStart = gasleft();
festivalPass.getUserMemorabiliaDetailed(user1);
uint256 gasEnd = gasleft();
console.log(gasStart);
console.log(gasEnd);
// the gas used turns out to be 14925713 which is around $54 in USD , which makes this prohibitively expensive to call
​
}

Recommended Mitigation

It is recommended to remove the getUserMemorabiliaDetailed function entirely.

The best practice is to handle this functionality off-chain:

Emit Detailed Events: Ensure that the MemorabiliaRedeemed event contains all necessary information (user, tokenId, collectionId, itemId) and that these fields are indexed.

- remove this code
+ add this code
Updates

Lead Judging Commences

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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