RabbitHole Quest Protocol contest - betweenETHlines's results

Lines of code

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

Vulnerability details

Impact

If the claim function runs out of gas, the caller can never claim any rewards without transferring the nfts to another address first

Proof of Concept

Currently, the claim function loops over the msg.senders NFT's. If this list ever becomes too large, the function will run out of gas.

Tools Used

VSCode

Recommended Mitigation Steps

Consider implementing pagination for reward claiming.

Lines of code

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

Vulnerability details

Impact

A replay attack could lead to undesired minting of NFTS from other privileged contracts and on other chains

Proof of Concept

Currently, the mintReceipt function mints one NFT to the msg.sender if the signature is valid. However, since the signature only consists of msg.sender and _questId, it becomes vulnerable for a replay attack.

Consider the following scenario:

The Rabbithole team decides to go crosschain with the same contracts. Alice got an approved signature for questId 1 on the origin chain. However, this signature is now also valid on the new chain.

Furthermore, this issue would also be present if a new QuestFactory is deployed and used.

Tools Used

VSCode

Recommended Mitigation Steps

Consider simply add the chainId and address(this) to the signature in order to reduce the risk of replay attacks. Moreover it might make sense to implement an expire timestamp.