I'm working on an NFT auction contract, which is an implementation of IERC721Receiver
and utilizes the onERC721Received
function to take action when a token is sent to the contract:
function onERC721Received(
address,
address _from,
uint256 _tokenId,
bytes calldata _data
)
external
returns (bytes4)
{
_createAuction(msg.sender, _from, _tokenId, _data);
return this.onERC721Received.selector;
}
My contract's onERC721Received
function is called by an NFT contract when the NFT contract's own safeTransferFrom
function is called by a token owner (or approved address) to transfer a token to my contract. My onERC721Received
function returns the expected response to the caller, and the NFT contract completes the transfer of the token to my contract's address.
This should all work as intended by the ERC721 specification.
However, as part of developing a robust contract, I'm dwelling ways that the onERC721Received
function could be used misused. I'm wondering how realistic these scenarios are, and what level of effort is appropriate to mitigate them.
Some of the scenarios I'm imagining are:
- The
onERC721Received
function can be spammed by non-contract addresses in order to create non-working auctions.
Since the function is external, it can be called by anyone. There is currently nothing preventing any user-owned address from calling it and passing in any data they want. The result would be an auction created in the contract (handled in my _createAuction
function) with erroneous data passed by a user.
Of course, a user would waste money spending gas fees to do this, and it would result in a non-working auction, with no benefit to them, but I'd still like to prevent the cleanup of any non-working auctions created this way.
The only way I can think to prevent this is to add a require(msg.sender.isContract(), 'Sender must be a contract address')
type of check to the function.
- The
onERC721Received
function can be spammed by other contracts in order to create non-workign auctions.
Similar ot the problem above, but in this scenario the call to onERC721Received
is called by a contract that is not compliant to the ERC721 standard. Erroneous data could be passed in the function call resulting in non-working auctions being created.
Again, this would not benefit the person trying to do this, and would only cost them gas fees, but its still a possibility.
I'm not sure what the solution to this one would be. Having an additional check to see if the msg.sender
address is ERC721 compliant? (although I believe this can be spoofed). Or, making sure a token transfer has actually taken place by calling ownerOf
on the NFT contract address?
Both of these scenarios seem like they aren't worth the effort of a bad actor, so I'm trying to gauge how concerned I should be about their possibility of happening. Any thoughts?