Is safeTransferFrom really safe?

ERC721 NFT contract offers two options for transferring NFTs: transferFrom and safeTransferFrom. You might wonder if safeTransferFrom is safer than transferFrom? The answer is yes and no. Let's break it down:

• safeTransferFrom's Safety: This method includes a check to ensure the recipient can correctly handle ERC721 NFTs. This means the NFT is less likely to become "stuck" if the recipient is a contract lacking the necessary NFT handling functionality.

• The Safety Caveat: While safeTransferFrom provides a safeguard for the NFT itself, it doesn't guarantee the safety of your contract. In fact, it could introduce a vulnerability to reentrancy attacks. This is because it triggers a function in the recipient contract if the recipient is another contract.

Let's dig into a real-world example. On January 25 2024, an attacker exploited a reentrancy vulnerability on NBLGAME contracts. The estimated total value of the exploited funds is approximately USD $180,000.

Overview

Attacker : https://optimistic.etherscan.io/address/0x1fd0a6a5e232eeba8020a40535ad07013ec4ef12 Attack Contract : https://optimistic.etherscan.io/address/0xe4d41bdd6459198b33cc795ff280cee02d91087b Vulnerable Contract : https://optimistic.etherscan.io/address/0x5499178919c79086fd580d6c5f332a4253244d91 Attack TX: https://optimistic.etherscan.io/tx/0xf4fc3b638f1a377cf22b729199a9aeb27fc62fe2983a65c4d14b99ee5c5b2328

Exploit Analysis

The vulnerability that was exploited stemmed from a reentrancy vulnerability in the withdrawNft function due to its use of safeTransferFrom when returning the NFT.

The attacker first deposited NFT then staked NBL tokens to the contract. While withdrawNFT, the contract sends the NFT back first, then returns the staked token later. Since safeTransferFrom triggers the attacker's contract's onERC721Received function, the attacker had a window of opportunity. The contract's state was inconsistent because the staked NBL amount had not yet been deducted. At this time, the amount of staked NBL is stored in amount so the attacker leveraged onERC721Received to initiate a reentrant call to withdrawNFT, effectively redepositing the NFT back to the contract to call withdrawNFT again.

The second withdrawal successfully retrieved the staked NBL tokens. Subsequently, the first withdrawal also deducted the NBL tokens from the previously stored amount, which was still intact due to the reentrancy. The exploit was success and the attacker received double the staked tokens.

Conclusion

When working with ERC721 NFT smart contracts, always aware that safeTransferFrom function will trigger onERC721Received in the recipient so they might form reentrancy vulnerable. Here are two effective ways to mitigate this risk:

• Checks-Effects-Interactions Pattern: This pattern emphasizes updating your contract's state variables before any external calls are made. This prevents an attacker from manipulating the contract's state during the external call.

• ReentrancyGuard: Consider using a ReentrancyGuard mechanism in all public/external functions. This adds a modifier that prevents functions from being re-entered before the initial call is complete.

For maximum security, it's recommended to apply both methods in your contracts whenever possible. While ReentrancyGuard incurs a small gas cost, it provides an additional layer of protection against reentrancy attacks.