code-423n4 / 2023-01-rabbithole-findings

1 stars 2 forks source link

Upgraded Q -> 2 from #621 [1675724705438] #695

Closed c4-judge closed 1 year ago

c4-judge commented 1 year ago

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

L2 - mintReceipt() function lacks a check to verify if the quest has already ended mintReceipt() function missing check for ended quest. This could result in a scenario where a receipt is minted after the quest has ended and the remaining tokens and fee have been withdrawn from the quest contract. This would result in a situation where there are more claimable receipts than there are tokens available in the quest balance to redeem them.

File: QuestFactory.sol 215: /// @dev mint a RabbitHole Receipt. Note: this contract must be set as Minter on the receipt contract 216: /// @param questId The id of the quest 217: /// @param hash The hash of the message 218: /// @param signature The signature of the hash 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: } Recommended Mitigation Steps

Consider adding check for quest not ended in mintReceipt() function:

function mintReceipt(string memory questId_, bytes32 hash_, bytes memory signature_) public { 
    if (IQuest(quests[questId_].address).ended < block.timestamp) revert QuestEnded(); 
    ...
}
c4-judge commented 1 year ago

kirk-baird marked the issue as duplicate of #22

c4-judge commented 1 year ago

kirk-baird marked the issue as satisfactory