Raisebox Faucet

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

Owner-triggered Denial of Service via burnFaucetTokens

Root + Impact

The burnFaucetTokens function is intended to allow the owner to burn a specified amount of faucet tokens. However, it is implemented in a way that transfers the entire token balance of the faucet contract to the owner before performing the burn operation. This completely drains the faucet of its tokens, rendering its primary function (claimFaucetTokens) unusable until the owner manually transfers tokens back or mints new ones. This constitutes a critical Denial of Service (DoS) vulnerability.

Description

  • Normal Behavior: The owner should be able to burn a specific amountToBurn amount from the faucet's token supply without affecting the remaining balance.

  • The Issue: The function incorrectly transfers the entire balance of the contract to the owner _transfer(address(this), msg.sender, balanceOf(address(this))); , regardless of the amountToBurn specified. This defeats the purpose of the faucet by removing all available tokens for claimers.

// Root cause in the codebase with @> marks to highlight the relevant section
function burnFaucetTokens(uint256 amountToBurn) public onlyOwner {
require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
// transfer faucet balance to the owner first before burning
// ensures owner has a balance before _burn (owner only function) can be called successfully
@> _transfer(address(this), msg.sender, balanceOf(address(this)));
_burn(msg.sender, amountToBurn);
}

Risk

Likelihood: Low

  • This action can only be performed by the contract owner.

  • The owner would likely trigger this by mistake while intending to burn only a small amount, not maliciously.

Impact: High

  • Denial of Service: The faucet becomes completely non-functional as its token balance drops to zero. No users can claim tokens.

  • Centralization Risk: The entire faucet supply is moved to the owner's EOA, centralizing all tokens and breaking the contract's intended operation.

Proof of Concept

A test case can demonstrate this behaviour. The owner calls burnFaucetTokens with a small amount (e.g., 1 token), but the faucet's entire balance is transferred to the owner, leaving the faucet with 0 tokens.

// This is a conceptual Foundry/Hardhat test
function test_burnDrainsFaucet() public {
// Setup: Faucet is deployed with INITIAL_SUPPLY
// Owner is vm.addr(1)
vm.prank(owner);
// Check initial faucet balance
uint256 initialFaucetBalance = faucet.balanceOf(address(faucet));
assertTrue(initialFaucetBalance > 0);
// Action: Owner calls burnFaucetTokens to burn just 1 token
uint256 amountToBurn = 1 * 10**18;
faucet.burnFaucetTokens(amountToBurn);
// Assert: The faucet's balance is now 0, and the owner has received all tokens.
uint256 finalFaucetBalance = faucet.balanceOf(address(faucet));
uint256 ownerBalance = faucet.balanceOf(owner);
assertEq(finalFaucetBalance, 0, "Faucet balance should be zero");
assertEq(ownerBalance, initialFaucetBalance - amountToBurn, "Owner should have the entire initial balance minus the burned amount");
}

Recommended Mitigation

The _burn function should be called directly on the contract's own balance. Since the ERC20 contract inherits from Context, _msgSender() within _burn will be address(this). However, _burn it is an internal function. The standard ERC20Burnable extension provides a public burn function, which should be used, but a direct internal call is appropriate since the contrct itself is the token. The logic should be changed to burn tokens directly from the contract's address without transferring them to the owner first.

function burnFaucetTokens(uint256 amountToBurn) public onlyOwner {
uint256 contractBalance = balanceOf(address(this));
require(amountToBurn <= contractBalance, "Faucet Token Balance: Insufficient");
-
- // transfer faucet balance to the owner first before burning
- // ensures owner has a balance before _burn (owner only function) can be called successfully
- _transfer(address(this), msg.sender, balanceOf(address(this)));
-
- _burn(msg.sender, amountToBurn);
+ _burn(address(this), amountToBurn);
}
Updates

Lead Judging Commences

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