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

Unintended Full Token Transfer to Owner in RaiseBoxFaucet::burnFaucetTokens()

Author Revealed upon completion

Unintended Full Token Transfer to Owner in RaiseBoxFaucet::burnFaucetTokens()

Description

  • The owner can calls burnFaucetTokens(uint256 amountToBurn) to burn certain amount of tokens (amountToBurn ), but the contract should send an amountToBurn to the owner before calling the inetrnal _burn().

  • The burnFaucetTokens(uint256 amountToBurn) transfers the entire faucet balance to the owner instead of the only specified amountToBurn. This means the owner ends up holding all faucet tokens, even though only part of them should 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:

  • Whenever the owner legitimately performs a token burn, this will always transfer all faucet tokens to their account first.

  • The function doesn’t enforce any logic to return leftover tokens to the faucet, so the state corruption persists indefinitely.

Impact:

  • Faucet token supply distribution breaks — faucet runs out of tokens, no user can claim.

Proof of Concept

Steps:

  1. Faucet holds INITIAL_SUPPLY amount of faucet tokens.

  2. Owner calls burnFaucetTokens()to burn amountToBurn.

  3. The amountToBurn from the faucet's balance was burnt, and the remaining unburnt balance was transferred to the owner, thus, the current faucet's balance is zero.

Add the following codes to RaiseBoxFaucet.t.sol and run forge test --mt testOwnerReceivesAllUnburntTokensAfterBurn :

function testOwnerReceivesAllUnburntTokensAfterBurn() public {
uint256 faucetInitialBalance = raiseBoxFaucet.getBalance(raiseBoxFaucetContractAddress); // 1000000000000000000000000000
uint256 ownerInitialBalance = raiseBoxFaucet.getBalance(owner); // 0
uint256 amountToBurn = 100000 * 10 ** 18; // 100000000000000000000000
​
vm.prank(owner);
raiseBoxFaucet.burnFaucetTokens(amountToBurn);
​
assertEq(raiseBoxFaucet.getBalance(raiseBoxFaucetContractAddress), 0);
assertEq(raiseBoxFaucet.getBalance(owner), ownerInitialBalance + (faucetInitialBalance - amountToBurn));
}

Result:

Ran 1 test for test/RaiseBoxFaucet.t.sol:TestRaiseBoxFaucet
[PASS] testOwnerReceivesAllUnburntTokensAfterBurn() (gas: 57909)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 184.06ms (26.58ms CPU time)
​
Ran 1 test suite in 546.56ms (184.06ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Burn the amountToBurn directly from the faucet's balance, i.e:

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

Lead Judging Commences

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