First Flight #10: One Shot

Summary

It is recommended to use safeTransferFrom() instead of transferFrom() when transferring ERC721s.

Vulnerability Details

The transferFrom() method is used instead of safeTransferFrom() in the stake(uint256 tokenId) function in streets.sol. I however argue that this isn’t recommended because:

OpenZeppelin’s documentation discourages the use of transferFrom(); use safeTransferFrom() whenever possible
The recipient could have logic in the onERC721Received() function, which is only triggered in the safeTransferFrom() function and not in transferFrom(). A notable example of such contracts is the Sudoswap pair:

Impact

While unlikely because the recipient is the function caller, there is the potential loss of NFTs should the recipient is unable to handle the sent ERC721s.

Tools Used

Manual Review

Recommendations

Use safeTransferFrom() when transferring the NFT to the streets.sol contract .