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

No NFT balance check in the `SantasList::buyPresent`

0xakira

Summary

There is no NFT balance check in the SantasList::buyPresent function in the SantasList.sol contract, allowing a duplicate NFT to be made.

Vulnerability Details

The vulnerability is in the SantasList::buyPresent function in the SantasList.sol contract starting at line 172.

function buyPresent(address presentReceiver) external {
i_santaToken.burn(presentReceiver);
_mintAndIncrement();
}

Impact

If a user with status EXTRA_NICE or NICE receives their NFT by calling the collectPresent() function, they can then call the buyPresent() function where they will also receive another NFT, thus creating a duplicate.

Tools Used

forge

Recommendations

Working Test Case

The following test creates a duplicate for a user with NICE status. When executed, this test will pass demonstrating that the user can get a duplicate NFT

function testCollectPresentNice() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NICE);
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
vm.prank(address(santasList));
santaToken.mint(user);
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(user);
santasList.collectPresent();
assertEq(santasList.balanceOf(user), 1);
santasList.buyPresent(user);
assertEq(santasList.balanceOf(user), 2);
console.log(santasList.balanceOf(user));
vm.stopPrank();
}

Run the test:

forge test --mt testCollectPresentNice -vvvv

Which yields the following output:

Running 1 test for test/unit/SantasListTest.t.sol:SantasListTest
[PASS] testCollectPresentNice() (gas: 144128)
Logs:
2
​
Traces:
[147143] SantasListTest::testCollectPresentNice()
├─ [0] VM::startPrank(santa: [0x70C9C64bFC5eD9611F397B04bc9DF67eb30e0FcF])
│ └─ ← ()
├─ [4251] 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)
│ └─ ← ()
├─ [0] VM::stopPrank()
│ └─ ← ()
├─ [0] VM::prank(SantasList: [0xE6E33783D6533ad50d373DDaa6fD21b272a57B69])
│ └─ ← ()
├─ [46713] SantaToken::mint(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], amount: 1000000000000000000 [1e18])
│ └─ ← ()
├─ [283] SantasList::CHRISTMAS_2023_BLOCK_TIME() [staticcall]
│ └─ ← 1703480381 [1.703e9]
├─ [0] VM::warp(1703480382 [1.703e9])
│ └─ ← ()
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← ()
├─ [70250] SantasList::collectPresent()
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], tokenId: 0)
│ └─ ← ()
├─ [678] SantasList::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 1
├─ [23399] SantasList::buyPresent(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ ├─ [2348] SantaToken::burn(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ │ ├─ emit Transfer(from: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], to: 0x0000000000000000000000000000000000000000, amount: 1000000000000000000 [1e18])
│ │ └─ ← ()
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], tokenId: 1)
│ └─ ← ()
├─ [678] SantasList::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 2
├─ [678] SantasList::balanceOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← 2
├─ [0] console::f5b1bba9(0000000000000000000000000000000000000000000000000000000000000002) [staticcall]
│ └─ ← ()
├─ [0] VM::stopPrank()
│ └─ ← ()
└─ ← ()
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.22ms
​
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

  • You should add a NFT balance check to presentReceiver and return error SantasList__AlreadyCollected() if the condition fails.

function buyPresent(address presentReceiver) external {
+ if (this.balanceOf(presentReceiver) == 0) {
i_santaToken.burn(presentReceiver);
_mintAndIncrement();
+ } else {
+ revert SantasList__AlreadyCollected();
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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