Root + Impact: Nested Loops in getUserMemorabiliaDetailed Can Lead to Prohibitive Transaction Costs and Cause Denial of Service with Growing Memorabilia Collections
Description
-
The FestivalPass contract provides the getUserMemorabiliaDetailed function to return arrays of token IDs, collection IDs, and item IDs for all memorabilia NFTs owned by a user, enabling off-chain applications to retrieve user-owned NFT details.
-
The function contains nested loops that iterate over all collection IDs (nextCollectionId) and item IDs (currentItemId) to check for user ownership, which can result in prohibitive gas costs or transaction failure (denial of service) as the number of memorabilia collections and items grows, exceeding the block gas limit.
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:
-
Occurs when the number of collections (nextCollectionId) or items per collection (currentItemId) is large, causing the nested loops to consume excessive gas, leading to prohibitive transaction cost and potentially exceeding the block gas limit.
-
Worsens as organizers create more collections or mint more items, which is likely in a popular festival with many memorabilia NFTs.
Impact:
-
Users and applications cannot retrieve memorabilia details due to transaction failures or prohibitive transaction costs, rendering the function unusable and preventing access to critical NFT ownership information.
-
Off-chain systems relying on this function (e.g., for displaying user NFTs) fail, reducing the utility of the memorabilia system and potentially impacting user trust and engagement.
Proof of Concept
Add the following function to test/FestivalPass.t.sol and run forge test --mt test_GetUserMemorabiliaDetailedDOS -vvv;
function test_GetUserMemorabiliaDetailedDOS() public {
vm.startPrank(organizer);
uint256[100] memory cols;
for (uint256 i = 0; i < 100; i++) {
uint256 col = festivalPass.createMemorabiliaCollection("Col", "ipfs://", 1e18, 10, true);
cols[i] = col;
}
vm.stopPrank();
vm.prank(address(festivalPass));
beatToken.mint(user1, 1000e18);
vm.startPrank(user1);
for (uint256 i = 0; i < cols.length; i++) {
for (uint256 j = 0; j < 5; j++) {
festivalPass.redeemMemorabilia(cols[i]);
}
}
vm.stopPrank();
uint256 gasBefore = gasleft();
festivalPass.getUserMemorabiliaDetailed(user1);
uint256 gasAfter = gasleft();
uint256 gasUsed = gasBefore - gasAfter;
console.log("Gas used by getUserMemorabiliaDetailed:", gasUsed);
}
Expected output:
Ran 1 test for test/FestivalPass.t.sol:FestivalPassTest
[PASS] test_GetUserMemorabiliaDetailedDOS() (gas: 45583338)
Logs:
Gas used by getUserMemorabiliaDetailed: 2205413
Observe excessive gas usage by getUserMemorabiliaDetailed function.
Recommended Mitigation
Add pagination functionality to getUserMemorabiliaDetailed function.
+ // Modified function with pagination to limit gas consumption
+ function getUserMemorabiliaDetailed(
+ address user,
+ uint256 startCollectionId,
+ uint256 maxCollections
+ ) external view returns (
+ uint256[] memory tokenIds,
+ uint256[] memory collectionIds,
+ uint256[] memory itemIds
+ ) {
+ uint256 endCollectionId = startCollectionId + maxCollections < nextCollectionId ? startCollectionId + maxCollections : nextCollectionId;
+ uint256 count = 0;
+ // Count memorabilia
+ for (uint256 cId = startCollectionId; cId < endCollectionId; cId++) {
+ for (uint256 iId = 1; iId < collections[cId].currentItemId; iId++) {
+ uint256 tokenId = encodeTokenId(cId, iId);
+ if (balanceOf(user, tokenId) > 0) {
+ count++;
+ }
+ }
+ }
+
+ // Populate arrays
+ tokenIds = new uint256[](count);
+ collectionIds = new uint256[](count);
+ itemIds = new uint256[](count);
+
+ uint256 index = 0;
+ for (uint256 cId = startCollectionId; cId < endCollectionId; 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);
+ }
- 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);
- }
