Malicious user can front run the selling of his NFT receipt with a claim() function call and stealing funds from another user

[code423n4]

Lines of code

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

Vulnerability details

Impact

Once the user has received an ERC-721 RabbitHole Receipt token, he has two options - either to call the claim() function and get the reward tokens, or sell it to another user who is interested. Here, a malicious user can cheat the system and do both. He does this by selling his receipt nft first and in the same block, front running the sell transaction with a call to claim() function.

Proof of Concept

The attack path works like this:

1. User completes an on-chain task and mints himself an ERC-721 RabbitHole Receipt token.
2. He creates a sell order for the same.
3. Another user checks that the receipt is not claimed yet and initiates the buy transaction.
4. Noticing the buy transaction in the mempool, the original malicious user calls the claim() function with a high gas fee (and front runs) and claims his reward tokens.
5. Right after, the buy transaction also goes through in the same block and the user was able to sell his already claimed(and hence useless) receipt.

Note that this works only when the buyer of the NFT manually checks if the receipt was not claimed and then initiates the buy transaction. If he uses a contract which does the check and buy in a single transaction, this attack path wont work.

Tools Used

VS code, Manual analysis

Recommended Mitigation Steps

The claim() function doesn't ask the user to send his receipt NFT to the contract before sending the reward tokens. It only checks if the user currently owns it and its not been claimed yet.

I suggest the claim() function should receive the NFT receipt before sending the reward tokens to avoid this kind of attack.

[c4-judge]

kirk-baird marked the issue as primary issue

[c4-sponsor]

waynehoover marked the issue as disagree with severity

[waynehoover]

This is an issue with whatever FE is displaying code from our smart contract, not an issue in the SC itself.

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

kirk-baird marked issue #119 as primary and marked this issue as a duplicate of 119