RabbitHole Quest Protocol contest - KIntern_NA's results

Lines of code

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

Vulnerability details

Impact

Function withdrawFee can be called by anyone and it can be repeated multiple times. Then all reward tokens can be drained easily from Quest contract to protocolFeeAddress by attacker. If some users dit not claim their rewards before, their rewards will be drained too.

//contract Erc20Quest.sol
function withdrawFee() public onlyAdminWithdrawAfterEnd {
    IERC20(rewardToken).safeTransfer(protocolFeeRecipient, protocolFee());
}

//contract Quest.sol
modifier onlyAdminWithdrawAfterEnd() {
    if (block.timestamp < endTime) revert NoWithdrawDuringClaim();
    _;
}

Proof of Concept

Here is the test file for this issue: https://gist.github.com/huuducsc/77e1c33a715bbcbd218edfec7ec0d991 Save this file into folder test/ and run yarn test to run it. It is based on the test file Erc20Quest.spec.ts, and repeats withdrawFee multiple times.

Tools Used

VS code Hardhat

Recommended Mitigation Steps

Set questFee = 0 after transfer fee

function withdrawFee() public onlyAdminWithdrawAfterEnd {
    IERC20(rewardToken).safeTransfer(protocolFeeRecipient, protocolFee());
    questFee = 0;
}

Lines of code

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

Vulnerability details

Impact

In contract Erc1155Quest, function withdrawRemainingTokens is used to withdraw all remaining tokens in the contract, even if these tokens are unclaimed from the minted receipts. It means after the owner calls withdrawRemainingTokens, users can not claim any rewards from their receipts.

Proof of Concept

//contract Erc1155Quest.sol
 function withdrawRemainingTokens(address to_) public override onlyOwner {
    super.withdrawRemainingTokens(to_);
    IERC1155(rewardToken).safeTransferFrom(
        address(this),
        to_,
        rewardAmountInWeiOrTokenId,
        IERC1155(rewardToken).balanceOf(address(this), rewardAmountInWeiOrTokenId),
        '0x00'
    );
}

Tools Used

VS code

Recommended Mitigation Steps

Follow the function withdrawRemainingTokens of contract Erc20Quest

Lines of code

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

Vulnerability details

Impact

In contract QuestFactory, function mintReceipt is used to mint a receipt by the hash message and signature for a quest. But because the hash message does not contain chainId and address(this), the signatures made on 1 chain can be simply reused in other chains with the same messages, and other deployed contracts QuestFactory can be reused the hash messages and signatures. Read EIP712 for more information

/// @dev recover the signer from a hash and signature
/// @param hash_ The hash of the message
/// @param signature_ The signature of the hash
function recoverSigner(bytes32 hash_, bytes memory signature_) public pure returns (address) {
    bytes32 messageDigest = keccak256(abi.encodePacked('\x19Ethereum Signed Message:\n32', hash_));
    return ECDSAUpgradeable.recover(messageDigest, signature_);
}

/// @dev mint a RabbitHole Receipt. Note: this contract must be set as Minter on the receipt contract
/// @param questId_ The id of the quest
/// @param hash_ The hash of the message
/// @param signature_ The signature of the hash
function mintReceipt(string memory questId_, bytes32 hash_, bytes memory signature_) public {
    if (quests[questId_].numberMinted + 1 > quests[questId_].totalParticipants) revert OverMaxAllowedToMint();
    if (quests[questId_].addressMinted[msg.sender] == true) revert AddressAlreadyMinted();
    if (keccak256(abi.encodePacked(msg.sender, questId_)) != hash_) revert InvalidHash();
    if (recoverSigner(hash_, signature_) != claimSignerAddress) revert AddressNotSigned();

    quests[questId_].addressMinted[msg.sender] = true;
    quests[questId_].numberMinted++;
    emit ReceiptMinted(msg.sender, questId_);
    rabbitholeReceiptContract.mint(msg.sender, questId_);
}

Proof of Concept

• On the original chain, Alice gets the signature of message hash contains Alice's address and questId after completing a quest.
• Alice uses these signature and message hash to mint receipt by function mintReceipt in the contract QuestFactory.
• When RabbitHole or any protocols deploy other contracts QuestFactory to use, Alice can reuse the above signature and message hash to call mintReceipt for quest questId, then claim rewards although Alice did not complete this quest.
• When RabbitHole is built on other chains, Alice can reuse this signature and message hash again.

Tools Used

VS code

Recommended Mitigation Steps

Follow the EIP712 to add the missing fields into the hash message (chainId, address(this))