RabbitHole Quest Protocol contest - romand's results

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/QuestFactory.sol#L222-L223

Vulnerability details

Impact

Quest completion signatures are signing a message that consists of (msg.sender, questId) tuple. The signature is in the form of an Ethereum signed message, and not of structured data as described in EIP712 https://eips.ethereum.org/EIPS/eip-712 https://github.com/rabbitholegg/quest-protocol/blob/main/contracts/QuestFactory.sol#L210-L213 In EIP712 structured data signing, the signature also signs a domain separator, which includes information such as app name, chain id, version, and address of verifying contract. Without the inclusion of the domain separator in the signature, the protocol is vulnerable to many attack vectors that the domain separator specification came to solve.

Ultimately this will allow signature reuse, where malicious actors can reuse previously signed signatures to get rewards without doing tasks

Proof of Concept

An example of a vulnerability can be signature re-use across chains. If a signature was signed in testnet for (sender bob, questId "XYZ"), then in mainnet deployment, if a quest with id "XYZ" will be created, bob can use the same signature to call mintReceipt and get rewards without doing the task.

* This vulnerability does require that the signer address claimSignerAddress will be the same in both chains, but this is entirely plausible and the documentation does not specify any restrictions about the claim signer.

* from tests such as https://github.com/rabbitholegg/quest-protocol/blob/main/test/ReceiptRenderer.spec.ts#L18 and lack of explanation in the documentation, we can assume that the questId will not include chain information.

Similarly, if claim signer signs a similar message (bob, "XYZ") for another protocol/Dapp, the signature can be re-used across protocols. The message format is simple enough, and without specification of questId formats (and tests such as https://github.com/rabbitholegg/quest-protocol/blob/main/test/Erc20Quest.spec.ts#L23) we can assume quests ids will be of simple, maybe numerical format, which can be duplicated in another protocol.

* This is less likely than the first vulnerability explained above, but can still happen

Other less likely vulnerabilities include examples such as if QuestFactory will have to be redeployed, perhaps to change the storage layout, then signatures from a previous deployment can be reused.

Tools Used

VS Code

Recommended Mitigation Steps

Implement EIP712 https://eips.ethereum.org/EIPS/eip-712