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

more than 1 NFT can be obtained per address

leogold

Summary

A malicous user can obtained more than one NFT per address

Vulnerability Details

Thou present are given to only nice and extranice users, A nice user can also be malicious and can advantage of the vulnerability in the collectPresent function , the balance of each user is what is used to determine if a user has collected a present which is actually not valid if a user transfer the NFT to another address in other to claim more NFT for nice user and more NFT and santaToken for Extra Nice user as much as they want to keep claiming present.

Impact

A malicious user can claim more NFT than expected by each user.

POC

function testCanCollectPresentIfAlreadyCollected() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NICE);
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
address userSecondAddress = makeAddr("userSecondAddress");
for (uint i = 0; i <= 200; i++) {
vm.startPrank(user);
santasList.collectPresent();
santasList.approve(userSecondAddress, i);
vm.stopPrank();
vm.prank(userSecondAddress);
santasList.transferFrom(user, userSecondAddress, i);
}
assertEq(santasList.balanceOf(userSecondAddress), 200);
}

The above code is a proof of concept of a user who claimed present 200 times and can also keep claiming for as long as possible
Add the above function to santaListTest.t.sol and run with forge test --mt testCanCollectPresentIfAlreadyCollected -vvvvv

Tools Used

manual review and foundry

Recommendations

Add a mapping to the stateVariable that keep track if a user has claimed a present or not i.e

//state variable
+ mapping(address -> bool) hasClaimedPresent;
​
- if (balanceOf(msg.sender) > 0) {
+ if (hasClaimedPresent[msg.sender]) {
revert SantasList__AlreadyCollected();
}
​
// add to the collectPresent function in the Nice logic and set the status of user who has claimed to true
+ hasClaimedPresent[msg.sender] = true;`
​
// add to the collectPresent function in the ExtraNice logic and set the status of user who has claimed to true
+ hasClaimedPresent[msg.sender] = true;
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.