White hat hacker samczsun: Attacks on NFT assets will become more frequent

Note: The original author is samczsun, a white hat hacker known as the "Audit God". He is also a research partner at Paradigm. He recently rescued $350 million in assets from the BitDAO MISO Dutch auction pool. In this article, he reminded of the potential security risks of NFT token standards. He also predicted that as ERC-721 and ERC-1155 token standards become more and more popular, attacks on NFTs are likely to become more frequent.

If you work in software engineering, chances are you have heard of at least one software engineering principle. While I don't advocate strict adherence to every single one of them, there are a few that are worth paying attention to.

What I'm going to talk about today is the Principle of Least Astonishment, which has a fancy name but is a very simple idea. What it says is that when presented with code that claims to do something, most users will make assumptions about how it accomplishes that thing. Therefore, as a developer, it's your job to write code that meets those assumptions so that your users aren't surprised.

This is a good principle to follow because developers love to make assumptions about things. If you export a function called calculateScore(GameState) a lot of people will assume that function will only read from the game state. If you also mutate the game state you'll expose a lot of people to confusing situations trying to figure out why their game state is randomly broken. Even if you put it in the documentation there's still no guarantee people will see it, so it's best to make sure your code isn't surprising in the first place.

"Six hours of debugging can save you five minutes of document reading."

The safer the better, right?

Back in early 2018, when the ERC-721 standard was being drafted, a proposal was made to implement transfer safety to ensure that tokens would not be stuck in a recipient contract that was not designed to handle tokens. To do this, the proposal authors modified the behavior of the transfer function to check if the recipient was able to support token transfers. They also introduced the unsafeTransfer function, which would bypass this check if the sender wished.

However, due to concerns about backward compatibility, this function was renamed in a subsequent commit. This made the transfer function for ERC-20 and ERC-721 tokens behave the same. However, the recipient check now needed to be moved somewhere else. Therefore, the standard authors introduced safe class functions: safeTransfer and safeTransferFrom.

This is a solution to a legitimacy problem, as there are many examples of ERC-20 tokens being accidentally transferred to contracts that never expected to receive tokens (a particularly common mistake is transferring tokens to a token contract, locking them up forever). It is not surprising that when drafting the ERC-1155 standard, the proposal authors took inspiration from the ERC-721 standard and incorporated receiver checks not only on transfers, but also on minting.

In the following years, these standards mostly lay dormant, while the ERC-20 token standard maintained its popularity, and the recent surge in gas costs, as well as the community's increased interest in NFTs, naturally led to developers increasingly using the ERC-721 and ERC-1155 token standards. With all this new interest, we should be thankful that these standards were designed with security in mind, right?

The safer the better, really?

Ok, but what does safe mean for transfers and minting? Different parties have different interpretations of safe. For developers, a safe function might mean that it does not contain any bugs or introduce additional security issues. For users, it might mean that it contains additional guardrails to protect them from accidentally shooting themselves in the foot.

It turns out that in this case, these functions are more of the latter and less of the former. This is particularly unfortunate because when choosing between the transfer and safeTransfer functions, why wouldn't you choose the safe one? It's in the name!

Well, one reason for this might be our old friend reentrancy, or what I’ve been working on renaming: unsafe external calls. Recall that any external call is potentially unsafe if the recipient is controlled by an attacker, as the attacker could cause your contract to transition to an undefined state. By design, these “safe” functions perform external calls to the recipient of the token, which is usually controlled by the sender during a mint or transfer. In other words, this is really a textbook example of an unsafe external call.

However, you might be asking yourself, what is the worst that could happen if receiving contracts were allowed to reject transfers they can’t process? Well, let me answer that question with two case studies.

Example 1: Hashmasks

Hashmasks is a limited supply NFT avatar project, and users can buy up to 20 mask NFTs per transaction (although they have been sold out for months). Here is the function to buy a mask:

You might think this function looks perfectly reasonable. However, as you might have expected, there is something sinister lurking within the _safeMint call. Let's take a look.

For security reasons, this function performs a callback to the recipient of the token to check if they are willing to accept the transfer. However, we are the recipient of the token, which means we have just been given a callback and can do whatever we want at this point, including calling the mintNFT function again. If we do this, we will call the function after only minting one mask, which means we can request to mint another 19 masks. This results in 39 mask NFTs being minted, even though the rules only allow a maximum of 20.

Example 2: ENS domain name wrapper

I was recently contacted by Nick Johnson from ENS who wanted to take a look at the work they are doing on the ENS Name Wrapper. This name wrapper allows users to tokenize their ENS names with a new ERC-1155 token, which provides support for fine-grained permissions and a more consistent API.

In summary, in order to wrap any ENS name (more specifically, all ENS names except 2LD.eth), you must first approve a name wrapper to access your ENS name. Then, you call wrap(bytes,address,uint96,address), which both mints an ERC-1155 token for you and manages the underlying ENS name.

Here is the wrap function, which is pretty simple. First, we call _wrap, which performs some logic and returns the hashed domain. Then, we make sure the sender of the transaction is indeed the owner of the ENS domain before taking over the domain. Note that if the sender does not own the underlying ENS domain, the entire transaction should be reverted, undoing any changes made in _wrap.

Below is the _wrap function itself, nothing special here.

Unfortunately, it is this _mint function that can provide unsuspecting developers with a terrible surprise. The ERC-1155 specification states that when a token is minted, the recipient should be consulted as to whether or not they are willing to accept the token. After digging into the library code (which was slightly modified from the OpenZeppelin base), we found that this is indeed the case.

But how exactly does this benefit us? Okay, once again we've seen an insecure external call that we can use to perform a reentrancy attack. Specifically, notice that during the callback, we have the ERC-1155 token for our tokenized ENS name, but the name wrapper has not yet verified that we own the underlying ENS name itself. This allows us to operate on the ENS name without actually owning it. For example, we can ask the name wrapper to unwrap our name, burn the token we just minted and get the underlying ENS name.

Now that we own the underlying ENS name, we can do whatever we want with it, like register new subdomains or set up resolvers. When we're done, we simply exit the callback. The name wrapper will interact with the current owner of the underlying ENS name (that's us) and complete the transaction. Just like that, we've taken temporary ownership of any ENS name that the name wrapper was approved for, and made any changes to it.

in conclusion

Surprising code can break things in catastrophic ways. In both cases, developers reasonably assumed that safe functions could be used safely, but inadvertently increased their attack surface. As the ERC-721 and ERC-1155 token standards become more popular and widespread, these types of attacks are likely to become more frequent. Developers need to consider the risks of using safe functions and determine how external calls interact with the code they write.