First Flight #5: Santa's List
First Flight #5
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

SantasList:_mintAndIncrement() mints NFT to msg.sender which results in SantasList:buyPresent() not working as expected

dougo

Summary

SantasList:_mintAndIncrement() mints NFT to msg.sender which results in SantasList:buyPresent() not working as expected, which is for the NFT to be minted to the intended recipient.

Vulnerability Details

Calling SantasList:buyPresent() will result in the caller ("gifter") obtaining an additional NFT, which is not the expected behavior. It is expected that the "recipient" will get the newly minted NFT.

Impact

High Overall (High impact, High likelihood)

PoC

The Foundry test below will fail due to this vulnerability on the assertion of recipient's NFT balance (if submitted finding for wrong address for token burn has been addressed -- if not, this will still fail due to arithmetic underflow)...

function testBuyPresentCorrected() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(user);
santaToken.approve(address(santasList), 1e18);
santasList.collectPresent();
santasList.buyPresent(recipient);
assertEq(santasList.balanceOf(user), 1);
assertEq(santasList.balanceOf(recipient), 1);
assertEq(santaToken.balanceOf(user), 0);
vm.stopPrank();
}

Tools Used

Visual Studio Code, Foundry

Recommendations

Add an overloaded function for SantasList:_mintAndIncrement() that takes a parameter representing the address to which the NFT will be minted as shown below...

function _mintAndIncrement(address to) private {
_safeMint(to, s_tokenCounter++);
}

Note: overloaded function is recommended as minimizes the amount of changes needed to address this issue. Alternately, you could change the original SantasList:_mintAndIncrement() function to take a parameter and change all existing calls to the function.

Next, change the SantasList:buyPresent() function as shown below to call the new overload function with parameter for address to mint NFT to...

function buyPresent(address presentReceiver) external {
i_santaToken.burn(presentReceiver);
_mintAndIncrement(presentReceiver);
}

Note: the function above does not reflect the change for submission related to incorrect address for burning token.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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