Upgraded Q -> 2 from #619 [1675724510983]

[c4-judge]

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

[L-02] The function mintReceipt should check if the quest has expired on-chain as well
The main function mintReceipt responsible for minting receipts lacks an important check to ensure the quest end time hasn't finished yet. Considering the fact that on quest creation every quest is enforced with a startTime and endTime, which represents the quest starting time and ending time. Users should not be allowed to mint receipts after the quest is expired.

By the sponsor comment, the claimSignerAddress takes care of that on the off-chain side and won't issue hashes before the quest start or after the quest ends. But mistakes always can occur and it is recommended to have a check on the smart contract level as well.

contracts/QuestFactory.sol

219: function mintReceipt(string memory questId_, bytes32 hash_, bytes memory signature_) public {
220: if (quests[questId_].numberMinted + 1 > quests[questId_].totalParticipants) revert OverMaxAllowedToMint();
221: if (quests[questId_].addressMinted[msg.sender] == true) revert AddressAlreadyMinted();
222: if (keccak256(abi.encodePacked(msg.sender, questId_)) != hash_) revert InvalidHash();
223: if (recoverSigner(hash_, signature_) != claimSignerAddress) revert AddressNotSigned();
224:
225: quests[questId_].addressMinted[msg.sender] = true;
226: quests[questId_].numberMinted++;
227: emit ReceiptMinted(msg.sender, questId_);
228: rabbitholeReceiptContract.mint(msg.sender, questId_);
229: }
Here is a recommended change, which takes care of this problem:

Add a storage variable in the struct Quest, which will hold the end time of the quest.
struct Quest {
mapping(address => bool) addressMinted;
address questAddress;
uint totalParticipants;
uint numberMinted;

•   uint256 expires;
  

  }
  When creating a quest with the function createQuest consider adding the endTime to the new stor variable expires.
  // Add the same check if contractType is erc1155 as well.

if (keccak256(abi.encodePacked(contractType_)) == keccak256(abi.encodePacked('erc20'))) {
if (rewardAllowlist[rewardTokenAddress_] == false) revert RewardNotAllowed();

        Erc20Quest newQuest = new Erc20Quest(
            rewardTokenAddress_,
            endTime_,
            startTime_,
            totalParticipants_,
            rewardAmountOrTokenId_,
            questId_,
            address(rabbitholeReceiptContract),
            questFee,
            protocolFeeRecipient
        );

        emit QuestCreated(
            msg.sender,
            address(newQuest),
            questId_,
            contractType_,
            rewardTokenAddress_,
            endTime_,
            startTime_,
            totalParticipants_,
            rewardAmountOrTokenId_
        );
        quests[questId_].questAddress = address(newQuest);
        quests[questId_].totalParticipants = totalParticipants_;

•       quests[questId_].expires = endTime_;
        newQuest.transferOwnership(msg.sender);
        ++questIdCount;
        return address(newQuest);
    }
  

And finally add a check in the function mintReceipt to check if the quest expired already.
function mintReceipt(string memory questId_, bytes32 hash_, bytes memory signature_) public {

•   if (quests[questId_].expires > block.timestamp) revert QuestAlreadyExpired();
    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_);
  

  }

[c4-judge]

kirk-baird marked the issue as duplicate of #22

[c4-judge]

kirk-baird marked the issue as satisfactory