Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Users can be tricked with claimed receipt tokens - Receipts can be claimed via flash loans #611

Closed
code423n4 opened this issue Jan 30, 2023 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-119 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L96-L118

Vulnerability details

The RabbitHoleReceipt token contract represents a receipt of a completed task for a quest. These tokens are minted for specific quests after a user successfully completes a task, and can be claimed just once using the claim function present in the Quest contract:

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++;
        }
    }

    if (redeemableTokenCount == 0) revert AlreadyClaimed();

    uint256 totalRedeemableRewards = _calculateRewards(redeemableTokenCount);
    _setClaimed(tokens);
    _transferRewards(totalRedeemableRewards);
    redeemedTokens += redeemableTokenCount;

    emit Claimed(msg.sender, totalRedeemableRewards);
}

This function queries the receipt token contract to determine which of the tokens the user holds was minted for that specific quest id. It marks the token ids as claimed and hands the rewards based on how many of those were unclaimed. The receipt token isn't burned or removed from the caller, it is just flagged as used internally in the quest contract.

Impact

A user can be tricked into buying a claimed receipt that doesn't entitle any reward. As receipt tokens are still in possession of the caller after they are claimed, a bad actor can claim the reward and still list the NFT in a secondary market.

In a similar way, another feasible attack would be using lending protocols. A bad actor can borrow or flash loan an unclaimed receipt, claim the rewards, and return it back.

PoC

First scenario:

  1. Attacker mints a receipt after completing a task or buys an unclaimed receipt.
  2. Attacker claims the rewards in the corresponding quest.
  3. Attacker lists the receipt token in a market.
  4. Victim buys the receipt token expecting a reward, though the receipt is already claimed.

Second scenario:

  1. Victim lists unclaimed reward token in exchange or lending platform.
  2. Attacker borrows or uses flash loans to get the receipt token.
  3. Attacker claims the rewards in the corresponding quest.
  4. Attacker returns the receipt token. Victim is left with a claimed receipt.

Recommendation

Given the current architecture of the solution, the most straightforward solution would be to either burn the receipt token or transfer it from the caller when the claim function is executed.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jan 30, 2023
code423n4 added a commit that referenced this issue Jan 30, 2023
@c4-judge c4-judge closed this as completed Feb 6, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Feb 6, 2023

kirk-baird marked the issue as duplicate of #201

@c4-judge
Copy link
Contributor

kirk-baird marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards duplicate-119 and removed duplicate-201 labels Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-119 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

2 participants