Raisebox Faucet

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

burnFaucetTokens drains faucet balance to owner

burnFaucetTokens drains faucet balance to owner

Description

  • Normal behavior: burning faucet tokens should decrease the token supply held by the faucet contract so tokens are permanently removed from circulation.

  • Specific issue: the owner-only burn function transfers the entire contract balance to the owner, then burns only amountToBurn from the owner. This leaves the owner holding the remaining tokens that were supposed to be burned.

function burnFaucetTokens(uint256 amountToBurn) public onlyOwner {
require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
// transfer faucet balance to 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: Medium

  • Owner calls the function during normal maintenance or to reduce supply, this will execute the logic and trigger the issue.

Impact: High

  • Contract token balance is entirely moved out: the call _transfer(address(this), msg.sender, balanceOf(address(this))) transfers the full faucet balance rather than the intended amount, instantly emptying the faucet's token holdings.

  • Burn semantics broken: burning only amountToBurn afterwards means the remainder ends up in the owner’s wallet

Proof of Concept

The PoC demonstrates that the owner can call burnFaucetTokens to transfer the faucet’s entire token balance to themselves and then burn only the requested amount, allowing the owner to siphon the remaining tokens.

Add the test below to the RaiseBoxFaucet.t.sol:

function testOwnerCanSiphonWithBurnBug() public {
// initial balances
uint256 contractBefore = raiseBoxFaucet.balanceOf(address(raiseBoxFaucet));
uint256 ownerBefore = raiseBoxFaucet.balanceOf(owner);
// owner requests to "burn" a small amount
uint256 amountToBurn = 1 * 10 ** 18;
vm.prank(owner);
raiseBoxFaucet.burnFaucetTokens(amountToBurn);
// balances after the call
uint256 contractAfter = raiseBoxFaucet.balanceOf(address(raiseBoxFaucet));
uint256 ownerAfter = raiseBoxFaucet.balanceOf(owner);
// PoC expectations for the bug:
// - contract balance is drained (transferred entirely to owner before burning)
// - owner ends up with (contractBefore - amountToBurn) additional tokens
assertEq(contractAfter, 0);
assertEq(ownerAfter, ownerBefore + (contractBefore - amountToBurn));
}

Run the test with:

forge test --match-test testOwnerCanSiphonWithBurnBug -vvv

Recommended Mitigation

  • Mitigation 1 -> minimal fix: transfer only the requested amount to owner before burning.

function burnFaucetTokens(uint256 amountToBurn) public onlyOwner {
require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
- _transfer(address(this), msg.sender, balanceOf(address(this)));
+ _transfer(address(this), msg.sender, amountToBurn);
_burn(msg.sender, amountToBurn);
}
  • Mitigation 2 -> (recommended): avoid sending tokens to owner at all and burn directly from the contract.

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);
+ _burn(address(this), amountToBurn);
}
Updates

Lead Judging Commences

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