code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The documentation explicitly mentions that the Receipts are to be issued to EOAs but no such checks available in the QuestFactory contract before minting a receipt to an address

Proof of Concept

Documentation:

Receipts: An NFT (ERC-721) representing a receipt from completing an action defined in the Quest. 
These are originally minted to a participants EOA address and the amount of available Receipts are defined by the Quest contract

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

As we can see no checks are included to ensure that the address calling the mintReceipt is an EOA and not a contract, an intended implementation mentioned in the documentation.

Tools Used

VS Code

Recommended Mitigation Steps

Additional check:

if(msg.sender!=tx.origin) revert suitableError();

c4-judge commented 1 year ago

kirk-baird changed the severity to QA (Quality Assurance)

c4-sponsor commented 1 year ago

jonathandiep marked the issue as sponsor disputed

jonathandiep commented 1 year ago

safeMint, which is used in RabbitHoleReceipt to mint should only allow mints to happens on EOAs