RabbitHole Quest Protocol contest - V_B's results
A protocol to distribute token rewards for completing on-chain tasks.
General Information
Platform: Code4rena
Start Date: 25/01/2023
Pot Size: $36,500 USDC
Total HM: 11
Participants: 173
Period: 5 days
Judge: kirk-baird
Total Solo HM: 1
Id: 208
League: ETH
Findings Distribution
Researcher Performance
Rank: 1/173
Findings: 2
Award: $8,821.89
🌟 Selected for report: 1
🚀 Solo Findings: 1
Findings Information
🌟 Selected for report: V_B
Labels
Awards
8803.1933 USDC - $8,803.19
External Links
Lines of code
https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L75 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L108
Vulnerability details
Description
The createQuest function deploys a quest contract using the create, where the address derivation depends only on the QuestFactory nonce.
At the same time, some of the chains (Polygon, Optimism, Arbitrum) to which the QuestFactory will be deployed are suspicious of the reorg attack.
Here you may be convinced that the Polygon has in practice subject to reorgs. Even more, the reorg on the picture is 1.5 minutes long. So, it is quite enough to create the quest and transfer funds to that address, especially when someone uses a script, and not doing it by hand.
Optimistic rollups (Optimism/Arbitrum) are also suspect to reorgs since if someone finds a fraud the blocks will be reverted, even though the user receives a confirmation and already created a quest.
Attack scenario
Imagine that Alice deploys a quest, and then sends funds to it. Bob sees that the network block reorg happens and calls createQuest. Thus, it creates quest with an address to which Alice sends funds. Then Alices' transactions are executed and Alice transfers funds to Bob's controlled quest.
Impact
If users rely on the address derivation in advance or try to deploy the wallet with the same address on different EVM chains, any funds sent to the wallet could potentially be withdrawn by anyone else. All in all, it could lead to the theft of user funds.
Recommended Mitigation Steps
Deploy the quest contract via create2 with salt that includes msg.sender and rewardTokenAddress_.
#0 - c4-judge
2023-02-06T23:26:45Z
kirk-baird marked the issue as unsatisfactory: Invalid
#1 - c4-judge
2023-02-06T23:26:57Z
kirk-baird changed the severity to QA (Quality Assurance)
#2 - vladbochok
2023-02-08T10:29:49Z
I do not agree with the resolution.
The project states that the Polygon is one of the networks where it will be deployed. As the attack scenario shows the reorg is not that rare.
Honest user makes two transactions:
- create quest A
- send funds to the quest A
An attacker notices the ongoing reorg and front runs the honest user transaction on creating the quest. So the final order of transaction is:
- Malicious user creates quest A
- Honest user creates quest B (earlier expect that deploy quest A)
- Honest user funds the quest A
#3 - vladbochok
2023-02-08T10:30:34Z
@c4-judge @kirk-baird
#4 - kirk-baird
2023-02-08T11:53:49Z
The recommendation would work, it would mean that address is deterministic and irrelevant of any reorgs.
The reason it works is because the to address when doing the token transfer will be irrelevant of the reorg.
An alternative solution could be to have the front end wait for certain number of blocks before between creating a quest and funding the quest.
#5 - vladbochok
2023-02-08T15:06:25Z
Thank you for your answer, @kirk-baird!
My main message is that this vulnerability is not "Invalid" or "unsatisfactory". This is more than a real bug (especially on the mentioned chains), and its probability greatly increases if the user uses the script for sequential deployment and financing of the quest. That's why I think it's nothing less than a medium-severity vulnerability.
#6 - c4-judge
2023-02-08T21:36:48Z
kirk-baird marked the issue as grade-b
#7 - c4-judge
2023-02-08T21:37:27Z
This previously downgraded issue has been upgraded by kirk-baird
#8 - c4-judge
2023-02-08T21:37:28Z
This previously downgraded issue has been upgraded by kirk-baird
#9 - kirk-baird
2023-02-08T22:09:50Z
Apologies I thought your first comment was saying the "Recommendations" are incorrect rather than the judging.
The initial mark of invalid was accidental and intended to be removed. The intended behaviour was to downgrade to QA. The reason for downgrading this was I considered it to be a front-end issue which would be protected by waiting for a certain number of block confirmations between each transaction. On reflection this does not seem to be a solid assumption and there is risk here due to the smart contract design.
The Warden has shown a good recommendation to prevent this kind of issue on the smart contract. Thanks for raising this, I'll upgrade it back to Medium.
#10 - c4-judge
2023-02-14T09:34:21Z
kirk-baird marked the issue as satisfactory
#11 - c4-sponsor
2023-04-11T20:01:42Z
waynehoover marked the issue as sponsor acknowledged
Findings Information
🌟 Selected for report: AkshaySrivastav
Also found by: KIntern_NA, SovaSlava, Tointer, Tricko, V_B, __141345__, betweenETHlines, bin2chen, cccz, critical-or-high, glcanvas, halden, hihen, jesusrod15, ladboy233, libratus, m9800, minhquanym, omis, peakbolt, rbserver, romand, rvierdiiev, wait, zaskoh
Awards
18.6976 USDC - $18.70
Labels
External Links
Lines of code
https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L222 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L223 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/QuestFactory.sol#L210
Vulnerability details
Description
In the mintReceipt function there is a check of the claimSignerAddress signature:
if (keccak256(abi.encodePacked(msg.sender, questId_)) != hash_) revert InvalidHash(); if (recoverSigner(hash_, signature_) != claimSignerAddress) revert AddressNotSigned();
The signature used in claimSignerAddress may be reused in other smart contracts. Basically, if the QuestFactory is deployed on different networks (Goerli/Ethereum mainnet/Arbitrum/Optimism/Polygon) the same signature will be valid and may be replayed.
Moreover, the signed message is that simple, which may have a collision with other dApps.
Attack scenario
The attacker sees the signature of the claimSignerAddress account on another chain/another contract and uses it to call mintReceipt function.
Impact
Let's assume that claimSignerAddress is the same for different instances of QuestFactory in different chains. Then an attacker may use the signature on a different chain for which the signature wasn't expected to be.
Recommended Mitigation Steps
Use the EIP-712 scheme to avoid signature replayability.
#0 - c4-judge
2023-02-06T23:25:53Z
kirk-baird marked the issue as duplicate of #45
#1 - c4-judge
2023-02-14T09:34:55Z
kirk-baird marked the issue as satisfactory