Santa's List
AI First Flight #3
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Severity: high
Valid

NFT Transferability Bypasses “One NFT Per Address” Rule

lallanaad

Root + Impact

The contract relies on checking whether a user’s current NFT balance is greater than zero to block repeat claims, but because ERC721 tokens are transferable this check can be bypassed simply by sending the NFT away, resetting the balance and allowing the user to mint again. This design flaw opens the door to repeated cycles of collecting, transferring, and recollecting, which lets users generate unlimited NFTs and SantaTokens, inflates supply far beyond what the system intended, breaks the fairness guarantee of one‑per‑address, and undermines trust in the project’s governance promises.

Description

The SantasList contract enforces the “one NFT per address” rule in SantaList::collectPresent by checking balanceOf(msg.sender) > 0. If the caller already owns an NFT, they cannot mint another. However, because the NFTs are fully transferable ERC721 tokens, a user can simply transfer their NFT away to another address, reducing their balance back to zero. This allows them to call SantaList::collectPresent again and mint additional NFTs.This breaks the intended invariant that each address should only ever receive one NFT, regardless of transfers.

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
@> if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}

Risk

Likelihood:

  • The contract only checks if a user currently owns an NFT.

  • Sending the NFT away resets the balance to zero.

Impact:

  • A single address can mint unlimited NFTs and Tokens and drain the contract.

Breaks the protocol invariant: “An address should only collect one NFT”.

Proof of Concept

  1. USER is marked NICE or EXTRA_NICE and calls SantaList::collectPresent .

  2. USER receives their NFT.

  3. USER transfers the NFT to USER2.

  4. USER’s balance is now zero.

  5. USER calls SantaList::collectPresent again.

  6. USER successfully mints a second NFT.

    Add below test code to the test contract

contract SantasListTest is Test {
SantasList santasList;
SantaToken santaToken;
​
address user = makeAddr("user");
address santa = makeAddr("santa");
address user2 = makeAddr("user2");
_CheatCodes cheatCodes = _CheatCodes(VM_ADDRESS);
​
function setUp() public {
vm.startPrank(santa);
santasList = new SantasList();
santaToken = SantaToken(santasList.getSantaToken());
vm.stopPrank();
​
// Advance time to after CHRISTMAS_2023_BLOCK_TIME
vm.warp(1703480382); // one second after the constant
​
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
}
​
function testCollectNFTtwice() public {
// Collect present First time
vm.startPrank(user);
santasList.collectPresent();
vm.stopPrank();
console.log("Balance Afer First:", santasList.balanceOf(user) );
​
// TRANSFER NFT FROM USER → USER2
vm.startPrank(user);
uint256 tokenId = 0;
santasList.safeTransferFrom(user, user2, tokenId);
vm.stopPrank();
console.log("Balance Afer Transfer:" , santasList.balanceOf(user) );
​
vm.startPrank(user);
// Collect present second time
santasList.collectPresent();
vm.stopPrank();
console.log("Balance Afer second:" , santasList.balanceOf(user) );
}
}

Recommended Mitigation

Replaces a balance-based check (balanceOf) with a state-based check using s_hasCollected

  • Prevents users from minting multiple NFTs by transferring them away

  • Enforces the “one NFT per address” invariant permanently

// Tracks whether an address has already collected a present.
​
+ mapping(address => bool) private s_hasCollected;
​
function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
​
- if (balanceOf(msg.sender) > 0) {
- revert SantasList__AlreadyCollected();
- }
// Enforce the "one NFT per address" invariant.
// Unlike balanceOf(msg.sender), this cannot be bypassed by transferring the NFT to another address.
+ if (s_hasCollected[msg.sender]) {
+ revert SantasList__AlreadyCollected();
+ }
​
if (
s_theListCheckedOnce[msg.sender] == Status.NICE &&
s_theListCheckedTwice[msg.sender] == Status.NICE
) {
_mintAndIncrement(msg.sender);
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE &&
s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement(msg.sender);
i_santaToken.mint(msg.sender);
} else {
revert SantasList__NotNice();
}
​
// Mark the user as having collected a present.
// This flag is intentionally set AFTER successful minting to avoid locking users out if the mint reverts.
+ s_hasCollected[msg.sender] = true;
}
​
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 5 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[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(); } ```

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!