Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Users eligible for token minting on call to `SantasList::collectPresent` can collect presents more than once.

Summary

Users eligible for token minting on call to SantasList::collectPresent can mint an unlimited number of tokens ( both SantaList erc721 and occasionally SantaToken erc20 )

Vulnerability Details

SantasList::collectPresent expects that at the moment of execution, the caller has no SantaList ERC721 tokens. A user can skirt this rule by transfering all of their current SantaList ERC721 tokens to another user prior to the call to SantasList::collectPresent and thus he can mint as many tokens as he wishes.

POC

`test/unit/SantasListTest.t.sol`
function testCantCollectPresentMoreThanOnce() 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);
santasList.collectPresent();
address userAddress2 = makeAddr("user_address_2");
santasList.transferFrom(user, userAddress2, 0);
santasList.collectPresent();
vm.stopPrank();
vm.prank(userAddress2);
santasList.transferFrom(userAddress2, user, 0);
assertGt(santasList.balanceOf(user), 1);
}

in the terminal run forge test --mt testCantCollectPresentMoreThanOnce

Impact

A user that is elligible to mint a SantasList ERC721 token or SantasList ERC721 token and SantaToken ERC20 token can mint as many of them as he wishes.

Tools Used

Manual review

Recommendations

Convert SantaList to an ERC721 snapshot token and only mint tokens to elligible users on execution of SantasList::collectPresent as per the state of SantasList at a particular snapshotId. This snapshotId should be saved for this user at the end of the first execution of SantasList::collectPresent for this user.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Weak Already Collected Check

Relying on balanceOf > 0 in collectPresent() allows the msg.sender to send their present to another address and then collect again.

Support

FAQs

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