A SITUATION IS POSSIBLE WHERE PROTOCOL DOESN'T GET ANY PROTOCOL FEES FOR MINTING RECEIPT

[code423n4]

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L81-L87
https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L219-L230

Vulnerability details

Impact

Protocol might not get any Fees for Mints done after End Time for a Quest whose hashes has been Issued in between start and End time.

Proof of Concept

File: QuestFactory.sol

    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_);
    }

Link to Code

As there are no checks on quest lifecycle, User can mint the Receipt at any time of their convenience. As number of hashes issued are tracked off chain, getting the Hash confirms its Eligibility to mint the Receipt.

So Consider the Following Situation:

1. User got the hash issued.
2. Once the End time is gone, Quest Owner withdraws all the Remaining Tokens. It also includes the Protocol fees of no. of users who didn't minted the Receipt.
3. Now few User whose hashes got issued earlier mints the token. They will successfully mint the token as max limit would not have reached.
4. But in this Case, Protocol losses on Protocol Fees which are already withdrawn by Quest owner.

Tools Used

Manual Review

Recommended Mitigation Steps

2 Ways to Mitigate the Issue depending on which functionality you consider more important:

1. Add an extra field in Quest Structure to track the End Time for the Quest and add a require condition in mintReceipt method such that no one is allowed to mint the Token after endTime.
2. Don't allow Owner to withdraw any amount from the Protocol Fees. Use maxProtocolReward instead of protocolFee to cover the Fees for Future mints.

File: Erc20Quest.sol

85:     uint256 nonClaimableTokens = IERC20(rewardToken).balanceOf(address(this)) - protocolFee() - unclaimedTokens;

Link to Code

[c4-judge]

kirk-baird marked the issue as duplicate of #22

[c4-judge]

kirk-baird changed the severity to 2 (Med Risk)

[c4-judge]

kirk-baird marked the issue as satisfactory