Unlimited NFT minting due to balance-based eligibility check
Unlimited NFT minting due to balance-based eligibility check
Description
The collectPresent function determines whether a user has already received their NFT by checking the current token balance of the caller:
This approach assumes that once a user receives an NFT, they will always hold it. However, NFTs are transferable by design. After successfully minting an NFT, a user can simply transfer it to another address. Once the balance becomes zero again, the same address can call collectPresent again and pass the balance check.
Because there is no additional state tracking to mark that an address has already claimed its present, this process can be repeated indefinitely, allowing the same user to mint an unlimited number of NFTs.
Risk
Likelihood: High
Exploiting this issue requires no special permissions, no timing assumptions, and no complex setup. Any user with basic knowledge of NFT transfers can repeatedly call collectPresent after transferring the minted NFT away. The attack is trivial to execute and highly likely to be abused once discovered.
Impact: High
An attacker can mint an unlimited number of NFTs, which can severely dilute the collection, break scarcity assumptions, and undermine the economic and reputational value of the project. If the NFTs have secondary market value or utility, this issue can directly lead to financial losses and loss of trust in the protocol.
Proof of Concept
To transfer an NFT, its tokenId must be known. In this protocol, token IDs are generated sequentially using the following state variable:
Since this variable has private visibility, it cannot be accessed directly from the contract interface. However, it can still be read from contract storage by identifying the storage slot in which it is stored.
To determine the storage slot, the following Foundry command can be used:
This command outputs the contract’s storage layout. From the output, we can observe that s_tokenCounter is stored in slot 8:
To demonstrate the exploit, an additional address is introduced in the test file to receive transferred NFTs:
Below is the full Proof of Concept test.
First, a helper function is used to read the current token ID by loading the value of s_tokenCounter directly from storage. Since the counter is incremented after minting, the actual token ID of the last minted NFT is s_tokenCounter - 1.
The main test demonstrates how a user can repeatedly mint NFTs by transferring them away between calls to collectPresent.
The test can be executed using:
Output:
As demonstrated by the test above, a user can mint multiple NFTs by repeatedly transferring the previously minted NFT to another address. This confirms that the balance-based eligibility check can be bypassed, allowing an attacker to mint an unlimited number of NFTs.
Recommended Mitigation
Introduce a dedicated mapping that records whether a user has already claimed a present:
This mapping should be used as the single source of truth for determining eligibility to collect a present.
Example Patch
Lead Judging Commences
[H-04] Any `NICE` or `EXTRA_NICE` user is able to call `collectPresent` function multiple times.
## Description `collectPresent` function is callable by any address, but the call will succeed only if the user is registered as `NICE` or `EXTRA_NICE` in SantasList contract. In order to prevent users to collect presents multiple times, the following check is implemented: ``` if (balanceOf(msg.sender) > 0) { revert SantasList__AlreadyCollected(); } ``` Nevertheless, there is an issue with this check. Users could send their newly minted NFTs to another wallet, allowing them to pass that check as `balanceOf(msg.sender)` will be `0` after transferring the NFT. ## Vulnerability Details Let's imagine a scenario where an `EXTRA_NICE` user wants to collect present when it is Christmas time. The user will call `collectPresent` function and will get 1 NFT and `1e18` SantaTokens. This user could now call `safetransferfrom` ERC-721 function in order to send the NFT to another wallet, while keeping SantaTokens on the same wallet (or send them as well, it doesn't matter). After that, it is possible to call `collectPresent` function again as ``balanceOf(msg.sender)` will be `0` again. ## Impact The impact of this vulnerability is HIGH as it allows any `NICE` user to mint as much NFTs as wanted, and it also allows any `EXTRA_NICE` user to mint as much NFTs and SantaTokens as desired. ## Proof of Concept The following tests shows that any `NICE` or `EXTRA_NICE` user is able to call `collectPresent` function again after transferring the newly minted NFT to another wallet. - In the case of `NICE` users, it will be possible to mint an infinity of NFTs, while transferring all of them in another wallet hold by the user. - In the case of `EXTRA_NICE` users, it will be possible to mint an infinity of NFTs and an infinity of SantaTokens. ``` function testExtraNiceCanCollectTwice() external { vm.startPrank(santa); // Santa checks twice the user as EXTRA_NICE santasList.checkList(user, SantasList.Status.EXTRA_NICE); santasList.checkTwice(user, SantasList.Status.EXTRA_NICE); vm.stopPrank(); // It is Christmas time! vm.warp(1_703_480_381); vm.startPrank(user); // User collects 1 NFT + 1e18 SantaToken santasList.collectPresent(); // User sends the minted NFT to another wallet santasList.safeTransferFrom(user, makeAddr("secondWallet"), 0); // User collect present again santasList.collectPresent(); vm.stopPrank(); // Users now owns 2e18 tokens, after calling 2 times collectPresent function successfully assertEq(santaToken.balanceOf(user), 2e18); } ``` ## Recommendations SantasList should implement in its storage a mapping to keep track of addresses which already collected present through `collectPresent` function. We could declare as a state variable : ``` mapping(address user => bool) private hasClaimed; ``` and then modify `collectPresent` function as follows: ``` function collectPresent() external { // use SantasList__AlreadyCollected custom error to save gas require(!hasClaimed[msg.sender], "user already collected present"); if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) { revert SantasList__NotChristmasYet(); } if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) { _mintAndIncrement(); hasClaimed[msg.sender] = true; return; } else if ( s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE && s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE ) { _mintAndIncrement(); i_santaToken.mint(msg.sender); hasClaimed[msg.sender] = true; return; } revert SantasList__NotNice(); } ``` We just added a check that `hasClaimed[msg.sender]` is `false` to execute the rest of the function, while removing the check on `balanceOf`. Once present is collected, either for `NICE` or `EXTRA_NICE` people, we update `hasClaimed[msg.sender]` to `true`. This will prevent user to call `collectPresent` function. If you run the previous test with this new implementation, it wail fail with the error `user already collected present`. Here is a new test that checks the new implementation works as desired: ``` function testCorrectCollectPresentImpl() external { vm.startPrank(santa); // Santa checks twice the user as EXTRA_NICE santasList.checkList(user, SantasList.Status.EXTRA_NICE); santasList.checkTwice(user, SantasList.Status.EXTRA_NICE); vm.stopPrank(); // It is Christmas time! vm.warp(1_703_480_381); vm.startPrank(user); // User collects 1 NFT + 1e18 SantaToken santasList.collectPresent(); // User sends the minted NFT to another wallet santasList.safeTransferFrom(user, makeAddr("secondWallet"), 0); vm.expectRevert("user already collected present"); santasList.collectPresent(); vm.stopPrank(); } ```
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.