RabbitHole Quest Protocol contest - Atarpara's results

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L102

Vulnerability details

Rabbit Hole C4 Contest

H-01 Owner can steal reward from unclaimed receipt

Impact

Owner can steal reward from the unclaimed receipt after the end quest and unclaimed receipt should not be able to claim their rewards

Proof of Concept

As per natspec docs on withdrawRemainingTokens() every receipt minted still able to claim their rewards and cann't be withdrawn owner but he can run withdrawFee() multiple times steal rewards from unclaimed receipt

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L79

    /// @notice Function that allows the owner to withdraw the remaining tokens in the contract
    /// @dev Every receipt minted should still be able to claim rewards (and cannot be withdrawn). This function can only be called after the quest end time
    /// @param to_ The address to send the remaining tokens to
    function withdrawRemainingTokens(address to_) public override onlyOwner {
    ...}

There are multiple scenario to leading this issue. I am explaining one of them.

For Example:

1. quest creator create one erc20 quest with totalParticipants_ = 10 , rewardAmountOrTokenId_ = 10e18, questFee=2000.
2. quest owner needs to transfer 120e18 reward token for starting quest (maxTotalRewards = 100e18, maxProtocolReward = 20e18)
3. Assume only 7 user mint mintReceipt for this contest, so receiptRedeemers() = 7 and protocolFee = 14e18 ( 7 *10e18*2000/10000)
4. Quest time is over and still yet claim only 5 users (50e18)

now contract balance is 120e18 - 50e18 = 70e18

5. Now owner decide to withdraw remaining tokens from the quest owner can sucessfully withdraw 36e18

Calculation :

unclaimedTokens = 20e18
nonClaimableTokens = 70e18(balance) - 14e18(protocol fee) - 20e18(unclaimedTokens) = 36e18 

contract balance is 34e18 (70e18 - 36e18)

6. Owner run withdrawFee() and transfer procotolfee ( 14e18)

contract balance is 20e18 (34e18-14e18) this balance only can be claim by unclaimed Receipt

7. but owner can run again withdrawFee() and transfer protocolfee(14e18)

Now contract balance is 6e18

8. 2 unclaimed receipt should able be claim their rewards due to insufficaint balance

Tools Used

Manual Review

Recommended Mitigation Steps

withdrawFee() and withdrawRemainingTokens() can call only once

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L99 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L117-L133

Vulnerability details

Impact

Quest.sol#Claim() can be lead into out of gas error due to unbounded array and user can't unable to claim reward token.

Proof of Concept

User can claim reward token via Receipt NFT contract. After claiming the reward token he still hold claimed mintReceipt and increase his balance. Also user can buy mint Receipt from other marketplace. getOwnedTokenIdsOfQuest() use 2 SLOAD opration and this gas cost is 2100*2 = 4200

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/RabbitHoleReceipt.sol#L117-L122

        for (uint i = 0; i < msgSenderBalance; i++) {
            uint tokenId = tokenOfOwnerByIndex(claimingAddress_, i);    // 1st SLOAD (2100 gas)
            if (keccak256(bytes(questIdForTokenId[tokenId])) == keccak256(bytes(questId_))) {       // 2nd SLOAD (2100 gas)
                tokenIdsForQuest[i] = tokenId;
                foundTokens++;
            }
        }

ethereum block gas limit is : 30_00_000

any user can lead into out of gas error after minting ~ 700 ( = 3000000/(2100*2) receipt lead into out of gas error and not able to claim reward token.

this limit is can be easily achievable.

Tools Used

Manual Review

Recommended Mitigation Steps

When user claim reward token Mint Receipt should be burn.