Raisebox Faucet

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

# Incorrect Token Transfer Before Burn Allows Owner to Drain Faucet Balance

TITLE (Root + Impact)

Incorrect Token Transfer Before Burn Allows Owner to Drain Faucet Balance (Low Impact)


Description

The burnFaucetTokens function in the RaiseBoxFaucet contract is intended to burn a specific amount of faucet tokens held by the contract. However, before burning, it transfers the entire token balance of the faucet to the owner, not just the amountToBurn.

As a result, the owner receives all tokens from the faucet’s balance while only burning a small portion of them. This behavior does not align with the documented purpose ("burn faucet tokens held by the contract") and can unintentionally drain the faucet’s token reserve.

Although the function is protected by onlyOwner, the logic error may cause operational issues or misuse of tokens if this function is called under the assumption it only burns tokens.


Severity

Low


Risk

  • Access Control: Proper (onlyOwner)

  • Impact: The faucet’s entire token supply can be transferred to the owner when only a portion should be burned.

  • Likelihood: Low (since only the owner can call), but could lead to misuse of funds or incorrect token accounting.


Impact

  • The faucet contract’s token balance can be entirely drained by the owner in one call.

  • The amountToBurn parameter becomes misleading since it does not control how many tokens are transferred.

  • The faucet’s functionality or accounting may break if other functions depend on its token balance.

  • Misleading documentation ("Burns faucet tokens held by the contract") creates risk for accidental misuse.


Tools Used

  • Manual Review

  • Foundry Unit Test (custom PoC confirming over-transfer)


Recommended Mitigation

Transfer and burn only the amount intended by the amountToBurn parameter. Add an event for transparency.

Fixed Version:

function burnFaucetTokens(uint256 amountToBurn) public onlyOwner {
require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
// Transfer and burn only the specified amount
_transfer(address(this), msg.sender, amountToBurn);
_burn(msg.sender, amountToBurn);
emit FaucetTokensBurned(msg.sender, amountToBurn);
}

Add an event definition:

event FaucetTokensBurned(address indexed owner, uint256 amount);

Proof of Concepts

Below is a self-contained Foundry test file that demonstrates the vulnerable and fixed behavior. You can place this single file in your test/ directory (e.g., test/BurnFaucetTokens.t.sol) and run forge test --match-path test/burnFaucetTokens.t.sol -vvv.

Note: This test contains both the vulnerable and fixed function variations in the same contract for demonstration and regression testing.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {Test, console} from "forge-std/Test.sol";
import {ERC20} from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import {Ownable} from "openzeppelin-contracts/contracts/access/Ownable.sol";
/// @title RaiseBoxFaucet - Simplified faucet with burn functionality for testing
contract RaiseBoxFaucet is ERC20, Ownable {
event FaucetTokensBurned(address indexed owner, uint256 amount);
constructor() ERC20("RaiseBox", "RBX") Ownable(msg.sender) {
_mint(address(this), 1_000_000 * 10 ** 18);
}
/// @notice Vulnerable version (transfers entire balance)
function burnFaucetTokens_Vulnerable(uint256 amountToBurn) public onlyOwner {
require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
_transfer(address(this), msg.sender, balanceOf(address(this))); // ❌ Transfers full balance
_burn(msg.sender, amountToBurn);
}
/// @notice Fixed version (transfers & burns exact amount)
function burnFaucetTokens_Fixed(uint256 amountToBurn) public onlyOwner {
require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
_transfer(address(this), msg.sender, amountToBurn); // ✅ Transfers only what's needed
_burn(msg.sender, amountToBurn);
emit FaucetTokensBurned(msg.sender, amountToBurn);
}
}
/// @title BurnFaucetTokensTest - Foundry test for RaiseBoxFaucet burn logic
contract BurnFaucetTokensTest is Test {
RaiseBoxFaucet faucet;
address owner = address(this);
function setUp() public {
faucet = new RaiseBoxFaucet();
}
/// @notice Demonstrates the bug in the vulnerable version
function test_BurnFaucetTokens_Vulnerable_Bug() public {
uint256 initialFaucetBalance = faucet.balanceOf(address(faucet));
uint256 burnAmount = 100 * 10 ** 18;
console.log("Initial faucet balance:", initialFaucetBalance / 1e18);
console.log("Burn amount:", burnAmount / 1e18);
faucet.burnFaucetTokens_Vulnerable(burnAmount);
uint256 ownerBalance = faucet.balanceOf(owner);
uint256 finalFaucetBalance = faucet.balanceOf(address(faucet));
console.log("Owner balance after burn:", ownerBalance / 1e18);
console.log("Faucet balance after burn:", finalFaucetBalance / 1e18);
// ❌ BUG: Owner receives full faucet balance minus burnAmount
assertEq(
ownerBalance,
initialFaucetBalance - burnAmount,
"Owner should wrongfully get full faucet balance minus burned tokens"
);
}
/// @notice Demonstrates the correct behavior after the fix
function test_BurnFaucetTokens_Fixed_CorrectBehavior() public {
uint256 initialFaucetBalance = faucet.balanceOf(address(faucet));
uint256 burnAmount = 100 * 10 ** 18;
console.log("\nInitial faucet balance:", initialFaucetBalance / 1e18);
console.log("Burn amount:", burnAmount / 1e18);
faucet.burnFaucetTokens_Fixed(burnAmount);
uint256 ownerBalance = faucet.balanceOf(owner);
uint256 finalFaucetBalance = faucet.balanceOf(address(faucet));
console.log("Owner balance after burn:", ownerBalance / 1e18);
console.log("Faucet balance after burn:", finalFaucetBalance / 1e18);
// ✅ Correct: Owner gets 0 balance after burn (transfer + burn of same amount)
assertEq(ownerBalance, 0, "Owner should have 0 tokens after burn");
assertEq(
finalFaucetBalance,
initialFaucetBalance - burnAmount,
"Faucet should lose exactly burnAmount tokens"
);
}
}

Notes

  • This finding is categorized as Low severity because the function is gated by onlyOwner. However, the logic is incorrect and can lead to accidental or intentional draining when the owner calls the function under incorrect assumptions.

  • The provided test is self-contained and demonstrates both the buggy and fixed behavior for regression testing.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 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.