Signature can be reused because of lacking domain in the hash

[code423n4]

Lines of code

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

Vulnerability details

Impact

In QuestFactory contract, users need to provide a signature signed by claimSignerAddress to mint a RabbitHole receipt. It has been seen that signed data is only msg.sender and questId_

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(); 
  // @audit signature replay on other chain

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

Because of lacking the domain of the contract that is going to consume the signature (chainId, contract address, version,...), the signature can be reused for another verification on another contract or different chain.

• Protocol deployed on more than 1 chain (e.g Ethereum and Polygon)
• Deploy new QuestFactory contract and signature can be used in both old and new version
• Hash value of completely different protocol that combine of (address, string) can also be used in QuestFactory

Proof of Concept

In mintReceipt() function, hash_ is only has msg.sender and questId_ as can be seen on the code snippet on Impact section.

In recoverSigner() function, there is no additional info about domain is added

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

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding domain of contract to prevent signature being used on different chain/contract

[c4-judge]

kirk-baird marked the issue as duplicate of #45

[c4-judge]

kirk-baird marked the issue as satisfactory