First Flight #11: Snek-Raffle
First Flight #11
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

`snek_raffle::fulfillRandomWords` Does Not Check Whether the Winner is Capable of Interacting with ERC-721 Tokens, Tokens can Get Stuck If Recipient is Unprepared

paprikrumplikas

Summary

snek_raffle::fulfillRandomWords mints and awards ERC-721 NFTs to raffle winners without verifying whether the recipient address (potentially a smart contract) is equipped to interact with ERC-721 tokens. This oversight can result in NFTs being permanently locked within contracts that lack the necessary interface to manage or transfer these tokens.

Proof of Code

The following smart contract acts as an unprepared recipient for the ERC-721 token: it lacks the onERC721Received function, making it unable to acknowledge or interact with received ERC-721 tokens properly.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
contract UnpreparedRecipient {
// Intentionally missing implementation of ERC721Receiver
​
function tryTransferToken(address nftContract, uint256 tokenId) external {
// Attempt to transfer the token, expecting this to fail
// because this contract cannot handle ERC-721 tokens.
}
}

and then use this test to demonstrate that this unprepared contract

from brownie import ERC721, UnpreparedRecipient, accounts, reverts
​
def test_minting_to_unprepared_contract():
# Setup: Deploy an ERC721 contract and UnpreparedRecipient contract
erc721 = ERC721.deploy("TestNFT", "TNFT", {'from': accounts[0]})
unprepared = UnpreparedRecipient.deploy({'from': accounts[1]})
​
# Act: Mint an NFT directly to the unprepared contract using mint
token_id = 1
erc721.mint(unprepared.address, token_id, {'from': accounts[0]})
​
# Assertion: Try to transfer the NFT from the unprepared contract should fail
# Since the unprepared contract lacks the means to interact with or transfer the NFT,
# this action is expected to revert. The precise revert message or behavior
# can depend on the ERC-721 implementation and the setup of the unprepared contract.
# The following is a conceptual assertion expecting a revert due to the lack of transfer capability.
with reverts():
erc721.transferFrom(unprepared.address, accounts[2], token_id, {'from': unprepared.address})
​

Impact

NFTs can be permanently locked within winner contracts that lack the necessary interface to manage or transfer ERC-721 tokens.

Tools Used

Brownie, ChatGPT

Recommendations

Use safeMint instead of mint:

- # ERC721.safe_mint, not using this
+ ERC721.safe_mint
​
function fulfillRandomWords(uint256 requestId, uint256[] memory randomWords) external {
...;
​
- ERC721._mint(recent_winner, ERC721._total_supply())
+ ERC721_safeMint(recent_winner, ERC721._total_supply());
​
// Continue with any additional logic following the minting process
...
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.