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

burnFaucetTokens() transfers entire contract balance to owner then burns only amountToBurn

cryptostellar5

Root + Impact

burnFaucetTokens(uint256 amountToBurn) currently transfers the entire token balance held by the faucet contract to the owner, then burns only amountToBurn from the owner’s balance. This leaves the owner with (contractBalance - amountToBurn) tokens instead of burning exactly amountToBurn from the contract.

Description

The function is intended to let the owner burn a specified amount of faucet tokens from the contract’s balance. Instead, the implementation transfers the contract’s full ERC20 token balance to the owner and then calls _burn(msg.sender, amountToBurn). This sequence results in only amountToBurn being removed from supply while most of the contract tokens end up in the owner’s wallet.

This behavior contradicts the parameter name. burnFaucetTokens(100) should burn 100 tokens from the faucet, not give the owner the remainder of the faucet balance.

function burnFaucetTokens(uint256 amountToBurn) public onlyOwner {
require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
@> _transfer(address(this), msg.sender, balanceOf(address(this)));
_burn(msg.sender, amountToBurn);

Risk

Likelihood:

  • The _transfer call moves the entire contract balance instead of the intended amountToBurn, then _burn removes only amountToBurn from the owner. The net effect: owner keeps balanceOf(contract) - amountToBurn.

Impact:

  • Owner (or attacker who controls the owner key) can move the contract’s entire token balance to themselves and burn only the requested amount - effectively exfiltrating almost all contract-held tokens.

Proof of Concept

function test_burnTransfersWholeBalance_thenBurnsOnlyAmount() public {
​
uint256 contractBalBefore = raiseBoxFaucet.balanceOf(address(raiseBoxFaucet));
uint256 ownerBalBefore = raiseBoxFaucet.balanceOf(address(this));
​
assertTrue(contractBalBefore > 0, "contract should have initial supply");
​
// Owner calls burn to burn only 100 tokens (use 1e18 units as per token decimals)
uint256 amountToBurn = 100 * 10 ** 18;
​
// call
raiseBoxFaucet.burnFaucetTokens(amountToBurn);
​
// after state
uint256 contractBalAfter = raiseBoxFaucet.balanceOf(address(raiseBoxFaucet));
uint256 ownerBalAfter = raiseBoxFaucet.balanceOf(address(this));
​
// Expectations under buggy implementation:
// 1) Contract balance becomes 0 (entire bal transferred)
// 2) Owner balance increases by contractBalBefore, then decreases by amountToBurn => ownerBalAfter == ownerBalBefore + contractBalBefore - amountToBurn
assertEq(contractBalAfter, 0, "Contract balance should be zero (bug: transferred whole balance)");
​
assertEq(
ownerBalAfter,
ownerBalBefore + contractBalBefore - amountToBurn,
"Owner balance should equal before + full contract bal - burned amount (bug)"
);
}

Run the above POC using the following command:

forge test -vvvv

Explanation:

  • contractBalBefore = X (e.g. initial supply minted to contract).

  • After call: contractBalAfter = 0 (bug: contract emptied).

  • ownerBalAfter = ownerBalBefore + X - amountToBurn (owner keeps nearly all tokens).

  • totalSupply reduced by only amountToBurn (not X).

Recommended Mitigation

  • Burn directly from the contract balance - no transfer to owner:

- _transfer(address(this), msg.sender, balanceOf(address(this)));
- _burn(msg.sender, amountToBurn);
+ // Burn directly from the contract's balance
+ _burn(address(this), amountToBurn);
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.