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

Inflexible Status Update Mechanism in checkTwice Function

luiscfaria

Summary

The SantasList smart contract has a significant logical flaw in the checkTwice function. This function enforces a rule that a user's status during the second check must match their status from the first check, not accommodating for legitimate status changes (upgrades or downgrades) between the two checks. This rigid design flaw prevents the system from recognizing and rewarding users whose behaviors have improved or declined.

Vulnerability Details

The issue arises from the checkTwice function's implementation. It includes a conditional statement that reverts the transaction if the status assigned during the second check (s_theListCheckedTwice) does not match the status from the first check (s_theListCheckedOnce). This condition overlooks legitimate scenarios where a user's behavior might warrant a change in status, such as upgrading from 'NICE' to 'EXTRA_NICE' or vice versa.

Impact

  • Inflexible Status Update: Users whose behavior merits a change in status between the two checks are unfairly locked into their initial categorization, denying them the opportunity to either receive enhanced rewards or face appropriate consequences for their actions.

Proof of Concept

Working test case

The following test uses the santa's address to set the users second status to EXTRA_NICE after updating to NICE. When run, the function will revert and this test will pass, demonstrating that a user cannot upgrade:

function test_Fail_CantUpgradeToExtraNice() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NICE);
vm.expectRevert();
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
}

Terminal:

Running 1 test for test/unit/SantasListTest2.t..sol:SantasListTest
[PASS] test_Fail_CantUpgradeToExtraNice() (gas: 18405)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.20ms

Tools Used

Foundry

Recommended Mitigation

Modify the checkTwice function to allow for status changes between the first and second checks.

function checkTwice(address person, Status status) external onlySanta {
if (s_theListCheckedOnce[person] != status) {
revert SantasList__SecondCheckDoesntMatchFirst();
}
s_theListCheckedTwice[person] = status;
emit CheckedTwice(person, status);
}

Let the collectPresent function handle the system's rules to see if a user should receive rewards.

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}
​
Status firstCheckStatus = s_theListCheckedOnce[msg.sender];
Status secondCheckStatus = s_theListCheckedTwice[msg.sender];
​
bool isNiceOrExtraNiceOnce = firstCheckStatus == Status.NICE ||
firstCheckStatus == Status.EXTRA_NICE;
bool isNiceOrExtraNiceTwice = secondCheckStatus == Status.NICE ||
secondCheckStatus == Status.EXTRA_NICE;
​
if (!isNiceOrExtraNiceOnce || !isNiceOrExtraNiceTwice) {
revert SantasList__NotNice();
}
​
_mintAndIncrement();
​
if (
firstCheckStatus == Status.EXTRA_NICE &&
secondCheckStatus == Status.EXTRA_NICE
) {
i_santaToken.mint(msg.sender);
}
}
Updates

Lead Judging Commences

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

Support

FAQs

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