code-423n4 / 2023-01-rabbithole-findings

1 stars 2 forks source link

Upgraded Q -> 2 from #670 [1675726386915] #702

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

Judge has assessed an item in Issue #670 as 2 risk. The relevant finding follows:

[L-03] DoS if address owns too many receipts With time it is viable for users to acquire thousands and tens of thousands of receipts. This may happen as a result of buying receipts for example, which was highlighted as a valid use-case. Moreover, receipts aren't burned when they are used for claiming a reward.

Calculations in getOwnedTokenIdsOfQuest require looping over all of user's tokens. This may lead to denial of service as EVM isn't suitable for big loops.

    uint msgSenderBalance = balanceOf(claimingAddress_);
    uint[] memory tokenIdsForQuest = new uint[](msgSenderBalance);
    uint foundTokens = 0;

    for (uint i = 0; i < msgSenderBalance; i++) {
        uint tokenId = tokenOfOwnerByIndex(claimingAddress_, i);
        if (keccak256(bytes(questIdForTokenId[tokenId])) == keccak256(bytes(questId_))) {
            tokenIdsForQuest[i] = tokenId;
            foundTokens++;
        }
    }

Recommendation: consider using ERC1155 for receipts which will allow tracking user receipts for each quest separately.

c4-judge commented 1 year ago

kirk-baird marked the issue as duplicate of #135

c4-judge commented 1 year ago

kirk-baird marked the issue as satisfactory