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

Unnecessary Token Transfer Before Burning in burnFaucetTokens

lochaeth

Root + Impact

Description

  • The function burnFaucetTokens(uint256 amountToBurn) transfers all faucet tokens from the contract to the owner before performing a burn.

  • However, the burn only affects amountToBurn, not the full transferred balance — meaning the contract sends all its tokens to the owner, and only a small portion is actually 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:

*The developer intended to ensure that the owner has a balance before calling _burn.

  • However, this logic is unnecessary because _burn can safely burn tokens directly from the contract’s own balance.
    Impact:

*The entire faucet token balance is transferred to the owner each time burnFaucetTokens is called.

  • The faucet becomes empty even if only a small portion of tokens was supposed to be burned.

Proof of Concept

function test_incorrectBurnFaucet() public {
vm.deal(address(raiseBoxFaucet), 1000 ether);
assertEq(address(raiseBoxFaucet).balance, 1000 ether, "Contract should have 1000 ether");
​
vm.prank(owner);
raiseBoxFaucet.burnFaucetTokens(100 ether);
assertFalse(address(raiseBoxFaucet).balance == 900 ether, "Contract balance should not be 900 ether after incorrect burn");
}

Recommended Mitigation

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 about 2 months 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.

Give us feedback!