Malicious drawingToken can make bad actor claim the raffle NFT

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-forgeries/blob/main/src/VRFNFTRandomDrawFactory.sol#L38-L51 https://github.com/code-423n4/2022-12-forgeries/blob/main/src/VRFNFTRandomDraw.sol#L277-L300 https://github.com/code-423n4/2022-12-forgeries/blob/main/src/VRFNFTRandomDraw.sol#L264-L274

Vulnerability details

Description

Malicious drawingToken address passed into factory.makeNewDraw() can claim the raffle NFT. It can be exploited via a social engineering attack or another scenario is that a malicious owner can pretend to make a raffle with a malicious drawingToken and claim himself the NFT. This supposed to be out of scope, but i thought it's still worthwhile to include it.

Impact

Loss of raffle NFT

Exploit Scenario

1. Eve the exploitor deploys a malicious fake collection that mimicks the BAYC NFT collection, it even has a similar contract address (first and last few characters are looking the same).
2. Eve mints herself 10 fake BAYCs.
3. Eve contacts Alice the admin and starts his social engineering campaign.
4. Eve acts as a BAYC whale and they chat for a while about NFTs, both enjoying the conversation.
5. Alice feels like he can trust Eve. Eve suggest to make a raffle for the BAYC collection and sends Alice info about the collection, about it's community, and the fake BAYC contract address.
6. Alice checks the info Eve sent including the contract address, but everything looks legit.
7. They talk while Alice deploys a new draw with calling factory.makeNewDraw() with Eve's contract address used for the drawingToken parameter.
8. Alice sets up the Chainlink cordinator and approves her targetNFT to use for the raffle contract and calls fulfillRandomWords() on the cordinator.
9. Eve's malicious drawingToken always returns her address when drawingToken.ownerOf() is called. The criteria for winning is hasUserWon(user) has to return true and that checks if the drawingToken.ownerOf() is the msg.sender aka the user called from winnerClaimNFT().
10. Eve calls winnerClaimNFT() and claims the raffle NFT.

Proof of Concept

MaliciousNFT used in VRFNFTRandomDraw.t.sol:

VRFNFTRandomDraw.t.sol:

Test result:

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using a multisignature wallet for creating raffles (for example 2/2 multisig: one deployer and one checker). Contract addresses should be triple checked before makeNewDraw deployment.

[hansfriese]

This sponsor mentioned clearly that any assumption on the token that is up for a raffle or used as drawing tokens will not be considered.

[c4-judge]

gzeon-c4 marked the issue as unsatisfactory: Out of scope