Root + Impact
Description
The normal behavior should let users query the memorabilia NFTs they own. The function should remain usable as the protocol grows.
getUserMemorabiliaDetailed() loops through every possible collection ID and every minted item ID in each collection, then repeats the same nested loop again to populate arrays. As collections and redeemed items grow, this view function can exceed RPC gas or memory limits and become unusable for users and frontends.
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 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 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:
-
This occurs as the organizer creates many memorabilia collections and users redeem many items over time.
-
This occurs for every query because the function scans global protocol state instead of only the user's owned memorabilia.
Impact:
Proof of Concept
function testGetUserMemorabiliaDetailedCostGrowsWithAllCollectionsAndItems() public {
address alice = makeAddr("alice");
vm.startPrank(organizer);
for (uint256 i = 0; i < 50; i++) {
festivalPass.createMemorabiliaCollection(
"Collection",
"ipfs://collection",
1e18,
100,
true
);
}
vm.stopPrank();
vm.prank(address(festivalPass));
beatToken.mint(alice, 10_000e18);
vm.startPrank(alice);
for (uint256 collectionId = 100; collectionId < 150; collectionId++) {
for (uint256 i = 0; i < 10; i++) {
festivalPass.redeemMemorabilia(collectionId);
}
}
vm.stopPrank();
uint256 gasBefore = gasleft();
festivalPass.getUserMemorabiliaDetailed(alice);
uint256 gasUsed = gasBefore - gasleft();
assertGt(gasUsed, 0);
}
The loop cost continues to grow with total collections and total items, and the function provides no pagination or bounded access path.
Recommended Mitigation
Store user-owned memorabilia token IDs when minting, then read from that per-user list. Alternatively, expose paginated query functions.
+mapping(address user => uint256[] tokenIds) private userMemorabilia;
function redeemMemorabilia(uint256 collectionId) external {
MemorabiliaCollection storage collection = collections[collectionId];
require(collection.priceInBeat > 0, "Collection does not exist");
require(collection.isActive, "Collection not active");
require(collection.currentItemId < collection.maxSupply, "Collection sold out");
BeatToken(beatToken).burnFrom(msg.sender, collection.priceInBeat);
uint256 itemId = collection.currentItemId++;
uint256 tokenId = encodeTokenId(collectionId, itemId);
tokenIdToEdition[tokenId] = itemId;
+ userMemorabilia[msg.sender].push(tokenId);
_mint(msg.sender, tokenId, 1, "");
}
+function getUserMemorabilia(address user) external view returns (uint256[] memory) {
+ return userMemorabilia[user];
+}
