transferfrom can lead to permanently loosing the NFT token. #16
Labels
3 (High Risk)
Assets can be stolen/lost/compromised directly
bug
Something isn't working
duplicate
This issue or pull request already exists
edited-by-warden
invalid
This doesn't seem right
Lines of code
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L192
Vulnerability details
Impact
transferfrom can lead to permanently loosing the NFT token.
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L192
Proof of Concept
Transferfrom doesn't ensure that the receiver is capable of receiving the token, which can lead to permanently loosing the token.
When using the transferFrom function of an ERC721 contract to send an NFT, if the receiving address is a smart contract and does not support ERC721, the NFT can be frozen in the contract. ERC721 has both safeTransferFrom and transferFrom. When using safeTransferFrom, the token contract checks to see that the receiver is an IERC721Receiver, which implies that it knows how to handle ERC721 tokens.
There is an explanation in the original Github ERC721 thread, which explains why they still keep the "unsafe" transfer() method.
ethereum/EIPs#721 (comment)
Tools Used
Manual review.
Recommended Mitigation Steps
Use ERC721's safeTransferFrom to ensure that the receiver can handle ERC721 tokens.
The text was updated successfully, but these errors were encountered: