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

Owner May Drain All Faucet Tokens via `burnFaucetTokens()`

fffuuuming

Root + Impact

Description

The burnFaucetTokens() function transfers the entire token balance of the contract to the owner before burning even if it just wants to burn a small amount of token

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 entire token balance of the contract
_transfer(address(this), msg.sender, balanceOf(address(this)));
​
_burn(msg.sender, amountToBurn);
}

Risk

Likelihood: High

  • Owner-only, so it will be used operationally

  • One call zeroes the faucet’s balance regardless of amountToBurn

Impact: Medium

  • DoS: faucet cannot serve future claims since its balance becomes 0

  • Operational hazard: routine “small burn” unintentionally wipes the faucet balance due to the full-balance transfer

Proof of Concept

Add the following test, then run this command: forge test --match-test testBurnFaucetTokensTransferAllTokens -vv

function testBurnFaucetTokensTransferAllTokens() public {
console.log("faucet token balance beofore burn 1 ether:", raiseBoxFaucet.getFaucetTotalSupply());
vm.prank(owner);
raiseBoxFaucet.burnFaucetTokens(1 ether);
console.log("faucet token balance after burn 1 ether:", raiseBoxFaucet.getFaucetTotalSupply());
}

PoC Results:

forge test --match-test testBurnFaucetTokensTransferAllTokens -vv
Ran 1 test for test/RaiseBoxFaucet.t.sol:TestRaiseBoxFaucet
[PASS] testBurnFaucetTokensTransferAllTokens() (gas: 53396)
Logs:
faucet token balance beofore burn 1 ether: 1000000000000000000000000000
faucet token balance after burn 1 ether: 0
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.89ms (1.29ms CPU time)
​
Ran 1 test suite in 331.40ms (8.89ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Remove the _transfer call. Instead, burn directly from the contract’s balance using _burn(address(this), amountToBurn).

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

Lead Judging Commences

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