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

[M-2] Burn method transfers the whole faucet token balance to the owner, completely emptying the faucet and blocking all subsequent token claims

cyberethdev

[M-2] Burn method transfers the whole faucet token balance to the owner, completely emptying the faucet and blocking all subsequent token claims

Description

  • Expected bahaviour The owner should be able to burn the token amount specified as input in the burnFaucetTokens method, leaving the rest of the token balance untouched in the faucet.

  • Problematic bahaviour The owner burns the token amount specified as input in the burnFaucetTokens method, but in the process transfers the whole token balance to themselves, leaving the faucet completely empty.

Root cause:

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: Medium

  • This bug occurs whenever the owner burns tokens via the burnFaucetTokens method.

Impact: Medium

  • The faucet stops processing claims entirely, breaking the faucet's intented functionality.

Proof of Concept

Add the following fuzz test to the Foundry test suite and run with forge test --mt test_burnFaucetTokens_TransfersAllTokens --fuzz-runs 1000.

  1. Get faucet contract balance before calling burn.

  2. Owner calls the burn function with an amount lower than the totak token balance.

  3. Get faucet contract balance again.

  4. Assert that there are no tokens left in the faucet even though the amount to burn was lower than the faucet balance.

  5. Assert that all subsequent calls to claimFaucetTokens revert.

function test_burnFaucetTokens_TransfersAllTokens(uint256 amount) public {
​
// Faucet token balance before burn
uint256 tokenBalanceBefore = raiseBoxFaucet.getFaucetTotalSupply();
​
// Burn token amount lower than the total token contract balance
vm.assume(amount < tokenBalanceBefore);
vm.startPrank(owner);
raiseBoxFaucet.burnFaucetTokens(amount);
vm.stopPrank();
​
// Faucet token balance after burn
uint256 tokenBalanceAfter = raiseBoxFaucet.getFaucetTotalSupply();
​
// Assert that burn function transfers the whole token balance from the contract
assertTrue(tokenBalanceAfter == 0, "Token balance is higher than zero");
​
// Assert that all token claims revert
vm.prank(user1);
vm.expectRevert(
abi.encodeWithSelector(RaiseBoxFaucet.RaiseBoxFaucet_InsufficientContractBalance.selector)
);
raiseBoxFaucet.claimFaucetTokens();
}

Recommended Mitigation

To mitigate this issue
A) Do not transfer the tokens to the owner at all, and burn the required amount directly from the contract's address:

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);
}

or alternatively, if the transfer to the owner is needed:
B) Transfer the amountToBurn instead of balanceOf(address(this)))

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);
}
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.