Raisebox Faucet

First Flight #50
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Valid

burnFaucetTokens sends full faucet balance to owner instead of burning specified amount

Root + Impact

Root Cause
burnFaucetTokens(uint256 amountToBurn) performs a pre-burn full-balance transfer from the faucet to the owner, then burns only amountToBurn. This ignores the parameter during transfer and drains faucet reserves.

Impact

  • Full faucet drain: The contract’s token reserves become 0 after any successful call.

  • Unintended owner enrichment: The owner keeps faucetBalance − amountToBurn, breaking faucet distribution and rendering the mechanism non-functional.

Description

Expected Behavior
A faucet “burn” should reduce supply by exactly amountToBurn from the faucet’s holdings, leaving the remaining faucet balance intact for user claims (or transfer exactly amountToBurn to the burner first, then burn).

Actual Behavior
The function transfers the entire faucet balance to the owner:

function burnFaucetTokens(uint256 amountToBurn) public onlyOwner {
require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
// @> BUG: pre-burn transfer uses the FULL faucet balance instead of `amountToBurn`
// @> This moves ALL reserves to the owner.
_transfer(address(this), msg.sender, balanceOf(address(this)));
// Burns only the specified amount from the owner's balance.
_burn(msg.sender, amountToBurn);
}

Total supply decreases by amountToBurn (which can mislead shallow tests), but the faucet reserves are already fully withdrawn to the owner.

Risk

  • Likelihood: Medium

    • Reason 1: A single privileged invocation by the owner (or anyone who controls the owner key) always produces a full drain; no special state required.

    • Reason 2: The pattern “transfer then burn” is common during maintenance; this flawed implementation will be executed during normal operations, not only under adversarial conditions.
      Impact: High

    • Impact 1: Funds at direct risk — faucet reserves go to zero in one call.

    • Impact 2: Protocol DoS / tokenomics disruptionclaimFaucetTokens reverts post-drain due to insufficient contract balance; users can no longer claim.
      Severity Recommendation: High (Impact: High; Likelihood: Medium due to onlyOwner)

Proof of Concept

Unit tests (Foundry) — file: test/BurnFaucetTokensDrain.t.sol

Key tests (all PASS against the vulnerable code):

  • test_BurnOneWei_DrainsAll()

    • Call burnFaucetTokens(1)

    • Observed:

      • balanceOf(faucet) == 0 (full drain)

      • balanceOf(owner) == initialFaucetBalance - 1 (owner windfall)

      • totalSupply == initialTotalSupply - 1

  • test_SmallBurn_DrainsFaucet()

    • Call burnFaucetTokens(1_000 ether)

    • Observed: faucet drained; owner receives faucetBalance - 1_000 ether; totalSupply decreases by 1_000 ether.

  • test_AfterDrain_ClaimIsNonFunctional()

    • After a drain, claimFaucetTokens() reverts with RaiseBoxFaucet_InsufficientContractBalance.

  • testFuzz_AnyAmount_DrainsFaucet(uint256 amount)

    • For any valid amount in [1, faucetBalance], faucet drains to zero and owner receives the windfall.

Illustrative trace excerpts (from forge test -vvvv):

emit Transfer(from: <faucet>, to: <owner>, value: 1e27) // full-balance transfer
emit Transfer(from: <owner>, to: 0x0, value: 1) // burn only amountToBurn
balanceOf(<faucet>) == 0 // faucet drained
balanceOf(<owner>) == 1e27 - 1 // unintended owner windfall
// SPDX-License-Identifier: MIT
pragma solidity 0.8.30;
import "forge-std/Test.sol";
import {RaiseBoxFaucet} from "../src/RaiseBoxFaucet.sol";
/**
* @title BurnFaucetTokensDrainTest
* @notice Security PoC for burnFaucetTokens() full-drain vulnerability.
*
* Summary:
* - Severity: High (funds at direct risk; Likelihood: Medium due to onlyOwner)
* - Type: Business Logic / Incorrect Amount Used / Funds Drain
* - Root Cause: Pre-burn transfer uses full faucet balance instead of `amountToBurn`
*
* Impact:
* - Faucet reserves become zero after any successful call.
* - Owner receives faucetBalance - amountToBurn (unintended windfall).
* - totalSupply decreases only by `amountToBurn` (masks the drain if you only check supply).
*
* Notes for reviewers:
* - This PoC is focused strictly on the burn function misbehavior.
* - See tests: small burn drains all, 1 wei drains all, faucet becomes non-functional for claim, fuzz (any amount).
*/
contract BurnFaucetTokensDrainTest is Test {
// ======== Constants (must align with contract) ========
string constant NAME = "RaiseBox";
string constant SYMBOL = "RBOX";
uint256 constant FAUCET_DRIP = 100 ether;
uint256 constant SEP_ETH_DRIP = 0.01 ether;
uint256 constant DAILY_SEP_ETH_CAP = 1 ether;
// From RaiseBoxFaucet: INITIAL_SUPPLY = 1_000_000_000 * 1e18
uint256 constant INITIAL_SUPPLY = 1_000_000_000 ether;
// Test burn scenarios
uint256 constant SMALL_BURN = 1_000 ether;
uint256 constant MEDIUM_BURN = 1_000_000 ether;
// ======== State ========
RaiseBoxFaucet faucet;
address owner;
address user;
// ======== Setup ========
function setUp() public {
faucet = new RaiseBoxFaucet(
NAME,
SYMBOL,
FAUCET_DRIP,
SEP_ETH_DRIP,
DAILY_SEP_ETH_CAP
);
owner = address(this); // test contract is the owner (Ownable(msg.sender))
user = makeAddr("user"); // valid EOA-like address via forge-std helper
// Sanity checks
assertEq(
faucet.balanceOf(address(faucet)),
INITIAL_SUPPLY,
"faucet initial balance mismatch"
);
assertEq(faucet.balanceOf(owner), 0, "owner should start with zero");
assertEq(faucet.totalSupply(), INITIAL_SUPPLY, "totalSupply mismatch");
assertEq(faucet.getOwner(), owner, "owner mismatch");
}
// ======== Snapshot helpers ========
struct BurnState {
uint256 faucetBalance;
uint256 ownerBalance;
uint256 totalSupply;
}
function _snap() internal view returns (BurnState memory s) {
s.faucetBalance = faucet.balanceOf(address(faucet));
s.ownerBalance = faucet.balanceOf(owner);
s.totalSupply = faucet.totalSupply();
}
// ======== Core PoC ========
/// @notice Small burn drains entire faucet and enriches owner.
function test_SmallBurn_DrainsFaucet() public {
BurnState memory before_ = _snap();
assertGt(
before_.faucetBalance,
SMALL_BURN,
"precondition: faucet > amountToBurn"
);
faucet.burnFaucetTokens(SMALL_BURN);
uint256 faucetAfter = faucet.balanceOf(address(faucet));
uint256 ownerAfter = faucet.balanceOf(owner);
uint256 supplyAfter = faucet.totalSupply();
// Faucet fully drained (BUG)
assertEq(
faucetAfter,
0,
"BUG: faucet should not be zero after a normal burn"
);
// Owner receives windfall (BUG)
assertEq(
ownerAfter,
before_.faucetBalance - SMALL_BURN,
"BUG: owner windfall mismatch"
);
// totalSupply looks correct (masks the bug)
assertEq(
supplyAfter,
before_.totalSupply - SMALL_BURN,
"supply must drop by amountToBurn"
);
}
/// @notice Burning 1 wei drains the entire faucet.
function test_BurnOneWei_DrainsAll() public {
BurnState memory before_ = _snap();
faucet.burnFaucetTokens(1);
assertEq(
faucet.balanceOf(address(faucet)),
0,
"BUG: 1 wei burn drained the faucet"
);
assertEq(
faucet.balanceOf(owner),
before_.faucetBalance - 1,
"BUG: owner took faucetBalance - 1"
);
}
/// @notice After drain, claim becomes non-functional.
function test_AfterDrain_ClaimIsNonFunctional() public {
// Drain first
faucet.burnFaucetTokens(SMALL_BURN);
// Prepare claim environment
vm.deal(address(faucet), 1 ether); // give faucet some ETH for drips (not essential here)
vm.warp(block.timestamp + 4 days); // pass cooldown
vm.prank(user);
// Expect revert due to insufficient token balance in faucet
vm.expectRevert(
RaiseBoxFaucet.RaiseBoxFaucet_InsufficientContractBalance.selector
);
faucet.claimFaucetTokens();
assertEq(faucet.balanceOf(address(faucet)), 0, "faucet remains empty");
}
/// @notice Edge: burning the entire balance leaves no windfall (all gets burned).
function test_BurnEntireBalance_EdgeCase() public {
uint256 bal = faucet.balanceOf(address(faucet));
faucet.burnFaucetTokens(bal);
assertEq(faucet.balanceOf(address(faucet)), 0, "faucet empty");
assertEq(
faucet.balanceOf(owner),
0,
"owner gets nothing in this edge case"
);
assertEq(faucet.totalSupply(), 0, "entire supply burned");
}
/// @notice Edge: burning more than balance reverts.
function test_BurnMoreThanBalance_Reverts() public {
uint256 bal = faucet.balanceOf(address(faucet));
vm.expectRevert("Faucet Token Balance: Insufficient");
faucet.burnFaucetTokens(bal + 1);
}
/// @notice Only owner can call burn.
function test_OnlyOwner_CanBurn() public {
address attacker = makeAddr("attacker");
vm.prank(attacker);
// For OZ v5 Ownable, this is the precise custom error; otherwise use vm.expectRevert();
vm.expectRevert(
abi.encodeWithSignature(
"OwnableUnauthorizedAccount(address)",
attacker
)
);
faucet.burnFaucetTokens(SMALL_BURN);
}
/// @notice Fuzz: any valid amount drains the faucet.
function testFuzz_AnyAmount_DrainsFaucet(uint256 amount) public {
uint256 bal = faucet.balanceOf(address(faucet));
amount = bound(amount, 1, bal);
BurnState memory before_ = _snap();
faucet.burnFaucetTokens(amount);
assertEq(faucet.balanceOf(address(faucet)), 0, "faucet always drained");
assertEq(
faucet.balanceOf(owner),
before_.faucetBalance - amount,
"owner always gets windfall"
);
}
}

Reproduce locally:

forge test -vvvv --match-path test/BurnFaucetTokensDrain.t.sol

Recommended Mitigation

Preferred fix — burn directly from the faucet contract

If a pre-transfer is strictly required by design, transfer only the amount to burn

Hardening (optional):

  • Emit event BurnFromFaucet(uint256 amount).

  • Use a dedicated role (e.g., BURN_MANAGER) rather than owner.

  • Add operational safeguards as needed (rate-limit, timelock, cap per tx).

- function burnFaucetTokens(uint256 amountToBurn) public onlyOwner {
- require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
- // transfer faucet balance to owner first before burning
- _transfer(address(this), msg.sender, balanceOf(address(this)));
- _burn(msg.sender, amountToBurn);
- }
+ function burnFaucetTokens(uint256 amount) public onlyOwner {
+ uint256 bal = balanceOf(address(this));
+ require(amount <= bal, "Faucet Token Balance: Insufficient");
+ // Burn directly from faucet reserves to avoid any pre-transfer windfall
+ _burn(address(this), amount);
+ }
Updates

Lead Judging Commences

inallhonesty Lead Judge 5 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Unnecessary and convoluted logic in burnFaucetTokens

Support

FAQs

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