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

Users can still collect present after the specified time

Summary

Users can collectPresent after Christmas(over 24 hours).

Vulnerability Details

The collectPresent() function only checks that presents cannot be collected before Christmas, but does not check that presents can be collected more then 24 hours after Christmas. So users can collectPresent after Christmas(over 24 hours).

According to the README.md: "The Christmas date is approximate, if it's more then 24 hours before or after Christmas, please report that."

So I think this is an issue.

PoC

Working Test Case:

function testCanCollectPresentMoreThen24hAfterChristmas() 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() + 1e10);
vm.startPrank(user);
assertEq(santasList.balanceOf(user), 0);
assertEq(santaToken.balanceOf(user), 0);
santasList.collectPresent();
assertEq(santasList.balanceOf(user), 1);
assertEq(santaToken.balanceOf(user), 1e18);
vm.stopPrank();
}

Add the test to the SantasListTest.t.sol file.
Running the test with forge test --match-test testCanCollectPresentMoreThen24hAfterChristmas -vvvv we can see:

Compiler run successful!
Running 1 test for test/unit/SantasListTest.t.sol:SantasListTest
[PASS] testCanCollectPresentMoreThen24hAfterChristmas() (gas: 191701)
Traces:
[191701] SantasListTest::testCanCollectPresentMoreThen24hAfterChristmas()
├─ [0] VM::startPrank(santa: [0x70C9C64bFC5eD9611F397B04bc9DF67eb30e0FcF])
│ └─ ← ()
├─ [24111] SantasList::checkList(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 1)
│ ├─ emit CheckedOnce(person: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], status: 1)
│ └─ ← ()
├─ [24419] SantasList::checkTwice(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 1)
│ ├─ emit CheckedTwice(person: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], status: 1)
│ └─ ← ()
├─ [0] VM::stopPrank()
│ └─ ← ()
├─ [283] SantasList::CHRISTMAS_2023_BLOCK_TIME() [staticcall]
│ └─ ← 1703480381 [1.703e9]
├─ [0] VM::warp(11703480381 [1.17e10])
│ └─ ← ()
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← ()
├─ [2678] SantasList::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 0
├─ [2542] SantaToken::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 0
├─ [113577] SantasList::collectPresent()
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], tokenId: 0)
│ ├─ [44713] SantaToken::mint(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], amount: 1000000000000000000 [1e18])
│ │ └─ ← ()
│ └─ ← ()
├─ [678] SantasList::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 1
├─ [542] SantaToken::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 1000000000000000000 [1e18]
├─ [0] VM::stopPrank()
│ └─ ← ()
└─ ← ()
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.82ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

This defeats the purpose of the protocol to begin with, and it is highly likely to happen.

Tools Used

Foundry

Recommendations

Add a check if it is over Christmas 24 hours.

Updates

Lead Judging Commences

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

collectPresent is callable after Christmas

check on block.timestamp only requires that christmas has arrived. The protocol explicitly states that after christmas has passed (give or take 24 hours) collecting shouldn't be possible.

Support

FAQs

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