Reflecting on the NBAxNFT drop failure

Failed NFT drops are all too common. It's important for developers to take a security-first approach when building decentralized applications.

A lot can be learned by studying what went wrong.

To that end, one of OpenZeppelin's security auditors took a quick peek at the smart contract code to help shed light on the vulnerabilities which led to a loss of nearly 18,000 NFTs in one hour.

For reference:

Examining the Code

The function “mint_approved” is how whitelisted addresses could mint in the presale. In order to mint, the “info” passed needs to be verified through the “verify” function.

A vulnerability enabled anyone to be considered whitelisted. Specifically, the contract reconstructed mint data without using msg.sender in place of the from data. This allowed users to sign with any valid signature (no matter whose) and then send TX as if they generated it.

In addition, users could have reused their own signatures, as the developers did not implement a nonce per address so a signature could only be used once.

The max a wallet could mint was 16,520, which is more than could be minted due to gas—the real number would be somewhere around 250.There is no check per use. Instead there is a global check to make sure the total amount minted in presale wasn't greater than "_MaxUserMintable"

After the checks in "mint_approved" pass, the code mints by calling "_mintCards". "numberOfCards" is a parameter sent as part of the TX (see 2nd parameter of "mint_approved"). As this amount is not part of the signed data that was checked, it could be manipulated to be anything.

The only check in "_mintCards" is making sure the total number of mints by all users (including the current user's request) hasn't minted more than the total amount available in the sale, "_MaxUserMintable", which we can find the value of using the function "tellEverything".

The output of current values from "tellEverything" using etherscan: https://etherscan.io/address/0xDD5A649fC076886Dfd4b9Ad6aCFC9B5eb882e83c#readContract

This is mapping the values from "tellEverything" to the variables they are.

Looking more deeply at the Sale contract code, it appears that OpenZeppelin Contracts were imported, but not used.

Looking more deeply at the Token contract, it appears that permissions could have been simplified by using OpenZeppelin’s AccessControl contract instead of the existing control, which used a “permitted” variable.

How to Ensure Success

An exploit like this could have been avoided by:

  • Importing and using unaltered code from OpenZeppelin Smart Contracts

  • Getting a smart contract audit

  • Using OpenZeppelin Defender

OpenZeppelin’s smart contract audits give Web3 developers valuable feedback to address security challenges of distributed systems. The report outlines potential problems in the code with a plan to guard against potential attack vectors and opportunities for improvement.

OpenZeppelin’s Defender Sentinels listen and react to events and function calls on your smart contracts across multiple networks, allowing you to monitor for potential exploits.

Your Turn

Feel free to check out the code for yourself and see what you find. What other changes would you suggest that could have prevented this drop from failing?