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

`SantasList:: collectPresent` Anybody can claim the reward even if he is not in the list

abhishekthakur

Summary

All users are by default has NICE status, so they can collect rewards without actually being added by the Santa.

Vulnerability Details

SantasList uses enum values for status NICE, EXTRA_NICE, NAUGHTY, NOT_CHECKED_TWICE.

enum Status {
NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}

As per current implementation,
0 represents NICE.
1 represents EXTRA_NICE
2 represents NAUGHTY
3 represents NOT_CHECKED_TWICE
These enum values are used as a mapping from address to status in the contract.

A user who is not in the list, Mapping will assign them first value, which is NICE.
enum order is not correct as per use case, due to which it makes the anybody a NICE status, due to that they can collectPresent which they are not supposed to collect.

POC

In existing test suite, add the following function -

function testCheckListWhoIsNotInList () public {
assertEq(uint256(santasList.getNaughtyOrNiceOnce(user)), uint256(SantasList.Status.NICE));
assertEq(uint256(santasList.getNaughtyOrNiceTwice(user)),uint256(SantasList.Status.NICE));
}

now run forge test --match-test testCheckListWhoIsNotInList -vv in the terminal and it will show results as follows

[⠢] Compiling...
[⠃] Compiling 1 files with 0.8.22
[⠊] Solc 0.8.22 finished in 1.71s
Compiler run successful!
​
Running 1 test for test/unit/SantasListTest.t.sol:SantasListTest
[PASS] testCheckListWhoIsNotInList() (gas: 13473)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.97ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Anybody can claim the reward alloted for NICE status

Tools Used

Foundry, Manual Review

Recommendations

reverse the enum order the way it's assigned to solve the issue.

enum Status {
- NICE,
- EXTRA_NICE,
- NAUGHTY,
- NOT_CHECKED_TWICE
​
+ NOT_CHECKED_TWICE,
+ NAUGHTY,
+ EXTRA_NICE,
+ NICE
}
Updates

Lead Judging Commences

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

default status is nice

In Solidity the first element of an enum is the default value. In Santa's List, the means each person is mapped by default to 'NICE'.

Support

FAQs

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