Santa's List
AI First Flight #3
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

No token burning approval mechanism

lordbolton

Root + Impact

Description

  • According to the standard ERC20 pattern, burning tokens from another address requires the token holder to first approve the burner to spend their tokens. The approval mechanism ensures token holders maintain control over who can transfer or burn their tokens, similar to how transferFrom() requires approval before moving tokens.

  • The SantaToken.burn() function burns tokens from an arbitrary from address without checking for approval or verifying that msg.sender has permission to burn those tokens. While restricted to calls from the SantasList contract, this creates a dangerous dependency where any bug or vulnerability in SantasList can result in unauthorized token burning.

// In SantaToken.sol
function burn(address from) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
@> _burn(from, 1e18); // No approval check - burns from 'from' address directly
}

Risk

Likelihood:

  • The SantasList contract calls this burn function whenever buyPresent() is invoked, directly burning tokens from the provided address parameter without any approval verification

  • Combined with the inverted logic in buyPresent(), this function enables immediate exploitation where attackers burn victims' tokens on every call

Impact:

  • Violates the ERC20 standard security pattern where token owners must explicitly approve others to spend or burn their tokens before any transfer occurs

  • Creates a systemic vulnerability where any bug in the SantasList contract (such as the inverted buyPresent() logic) can result in unauthorized burning of user tokens without their consent

  • Users lose control over their own tokens, as they can be burned without approval through any vulnerable function in SantasList that calls burn()

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import {Test} from "forge-std/Test.sol";
import {SantasList} from "../../src/SantasList.sol";
import {SantaToken} from "../../src/SantaToken.sol";
​
contract ExploitBurnNoApprovalTest is Test {
SantasList santasList;
SantaToken santaToken;
address santa = makeAddr("santa");
address user = makeAddr("user");
function setUp() public {
vm.prank(santa);
santasList = new SantasList();
santaToken = SantaToken(santasList.getSantaToken());
}
function testBurnWithoutApproval() public {
// Setup: Give user tokens
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.prank(user);
santasList.collectPresent();
assertEq(santaToken.balanceOf(user), 1e18);
// User has NOT approved SantasList to spend their tokens
assertEq(santaToken.allowance(user, address(santasList)), 0);
// But SantasList can still burn user's tokens via buyPresent
vm.prank(user);
santasList.buyPresent(user); // Burns user's tokens without approval!
assertEq(santaToken.balanceOf(user), 0);
}
function testStandardERC20PatternComparison() public {
// In a standard ERC20 implementation with proper approval:
// 1. User approves SantasList: token.approve(santasList, amount)
// 2. SantasList checks allowance before burning
// 3. Burn fails if allowance is insufficient
// Current implementation skips approval entirely, violating ERC20 security model
}
}

Recommended Mitigation

+ // Option 1: Require approval before burning
function burn(address from) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
+ // Check that the from address has approved SantasList to burn their tokens
+ uint256 allowed = allowance[from][msg.sender];
+ require(allowed >= 1e18, "Insufficient allowance");
+ allowance[from][msg.sender] = allowed - 1e18;
_burn(from, 1e18);
}
​
+ // Option 2: Only allow burning msg.sender's tokens (requires refactoring SantasList)
- function burn(address from) external {
+ function burn() external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
- _burn(from, 1e18);
+ // SantasList must pass the actual buyer through, not arbitrary address
+ _burn(tx.origin, 1e18); // Note: tx.origin has security concerns, better to refactor
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 14 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!