safeTransferFrom.md

A Not So Safe safeTransferFrom Implementation

Hi all! It has been awhile since I've written a blog post. I've been researching smart contracts and creating my own game that integrates NFTs or (ERC 721) tokens. During this time I discovered a way to empty all Ethereum of the owner of a ERC 721 factory contract.

OpenZeppelin provides some awesome example contracts the one that I'm using as an example is located here https://docs.openzeppelin.com/contracts/3.x/erc721.

// contracts/GameItem.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.6.0;

import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import "@openzeppelin/contracts/utils/Counters.sol";

contract GameItem is ERC721 {
   using Counters for Counters.Counter;
   Counters.Counter private _tokenIds;

   constructor() public ERC721("GameItem", "ITM") {}

   function awardItem(address player, string memory tokenURI)
       public
       returns (uint256)
   {
       _tokenIds.increment();

       uint256 newItemId = _tokenIds.current();
       _mint(player, newItemId);
       _setTokenURI(newItemId, tokenURI);

       return newItemId;
   }
}

It's important that we go over how imports work in solidity and what some of the function modifiers mean.

The imports

import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import "@openzeppelin/contracts/utils/Counters.sol";

After importing another solidity contract into the main contract we are able to access all functions of the imported contract as long as they are exported or public. We are going to be focusing on the interface ERC721.sol.

---

The modifiers

There aren't many modifiers in this factory contract. The contructor for GameItem.sol is public so anyone from anwyhere can call the imported functions from this ERC721.sol interface contract.

constructor() public ERC721("GameItem", "ITM") {}

Key takeaways

public - All can access.

Now that we know that the constructor is public we can search for interesting imported functions.

Today we will be focusing on safeTransferFrom. Here is the safeTransferFrom specification from https://eips.ethereum.org/EIPS/eip-721

function safeTransferFrom(address _from, address _to, uint256 _tokenId) external payable;

   /// @notice Transfer ownership of an NFT -- THE CALLER IS RESPONSIBLE
   ///  TO CONFIRM THAT `_to` IS CAPABLE OF RECEIVING NFTS OR ELSE
   ///  THEY MAY BE PERMANENTLY LOST
   /// @dev Throws unless `msg.sender` is the current owner, an authorized
   ///  operator, or the approved address for this NFT. Throws if `_from` is
   ///  not the current owner. Throws if `_to` is the zero address. Throws if
   ///  `_tokenId` is not a valid NFT.
   /// @param _from The current owner of the NFT
   /// @param _to The new owner
   /// @param _tokenId The NFT to transfer
   

This function has the declaration of external payable these two modifiers combined can be risky.

Key takeaways

external - Cannot be accessed internally, only externally.

payable - Ether can be sent to the function with this modifier, and ether will also be used during the transaction.

The safeTransferFrom function has a lot of safety checks built in such as making sure the msg.sender is the owner of the token and that the NFT exists before allowing the transaction with the Ethereum network.

/// @param _from The current owner of the NFT
/// @param _to The new owner
/// @param _tokenId The NFT to transfer

The vulnerability

This raises the question though, what if the owner is also the one receiving the nft? Turns out you most definitely need a check to ensure that someone doesn't try to exhaust all resources maliciously from the owner of the NFTs by sending a safeTransferFrom transaction back to the owner in a loop.

Here's an example with Web3.py showing how this can happen with an app reading a json POST request variable.

receiver_address = post_data['receiver_eth_address']
test = contract.functions.safeTransferFrom(dev, receiver_address, random_nft).buildTransaction({'gasPrice': gas_price, 'nonce': web3.eth.getTransactionCount(dev, 'pending')})

So now all that's needed to send a token back to the owner is finding out the owner's public eth address, easy enough with etherscan.io. Sending a POST request with the owner's address as the _to field would effectively send a a token from the owner back to the owner. Oh no there goes some of my rinkeby test ether!

https://rinkeby.etherscan.io/tx/0x56288b0d1e0a36ecec7f97f792033e0d2b98550607f1abbbe0013ef61b2a620e

---

Fix

This issue can be fixed by checking that the receiving address does not equal the owner address.

Example with Python.

receiver_address = post_data['receiver_eth_address']

if receiver_address != dev:

	test = contract.functions.safeTransferFrom(dev, receiver_address, random_nft).buildTransaction({'gasPrice': gas_price, 'nonce': web3.eth.getTransactionCount(dev, 'pending')})

As you can imagine not checking the receiver address could be very costly on main net. So stay safe, ethical, and classy people of the internet. Until next time!

---

Resources

https://docs.openzeppelin.com/contracts/3.x/erc721

https://eips.ethereum.org/EIPS/eip-721

If you enjoyed this blog post and want to support my work here's my Ethereum address 0x034c06c0d32c7D5362B9A850B560a2923ffe8aCb.