Upgraded Q -> 2 from #648 [1675725337760]

[c4-judge]

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

2. Unbounded Array Vulnerability in Claim Function Link : https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L96

Summary:

The claim function in the Quest contract has an unbounded array vulnerability that could lead to an Out-of-Gas (OOG) error and make the contract unresponsive. The vulnerability occurs because the setClaimed function is being called with an arbitrarily long tokenIds array, which could consume all the gas in the block, causing the transaction to fail.

Impact:

An attacker could submit a malicious transaction that contains a long array, causing the claim function to run out of gas, and making the contract unresponsive. This could result in a Denial-of-Service (DoS) attack on the contract.

Recommendation: To prevent this vulnerability, the size of the tokenIds array should be limited to a reasonable number of items. For example, you could limit the length of the tokenIds array to 256 items. Additionally, you should perform a gas check before calling the _setClaimed function to ensure that there is enough gas available to complete the transaction.

Example:

// Limit the size of the tokenIds_ array to 256 items function claim() public virtual onlyQuestActive { if (isPaused) revert QuestPaused();

uint[] memory tokens = rabbitHoleReceiptContract.getOwnedTokenIdsOfQuest(questId, msg.sender);

if (tokens.length == 0) revert NoTokensToClaim();

uint256 redeemableTokenCount = 0;
for (uint i = 0; i < tokens.length; i++) {
    if (!isClaimed(tokens[i])) {
        redeemableTokenCount++;
    }
}

// Check if there is enough gas to complete the transaction
require(redeemableTokenCount <= 256, "Array size is too large");

uint256[] memory tokenIds = new uint256[](redeemableTokenCount);
uint256 j = 0;
for (uint i = 0; i < tokens.length; i++) {
    if (!isClaimed(tokens[i])) {
        tokenIds[j] = tokens[i];
        j++;
    }
}

_setClaimed(tokenIds);

[c4-judge]

kirk-baird marked the issue as duplicate of #135

[c4-judge]

kirk-baird marked the issue as satisfactory