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

Burning tokens result in transferring all tokens from the contract to the owner

jjhokins

Root + Impact

Description

  • The function "burnFaucetTokens" is implemented with a major flaw: once the function is trigged, ALL balance of the token contract is transferred to the owner, leaving the token contract itself with 0 tokens, preventing all users from further claiming any faucet drips.
    Then the intended amount of token is burned with the rest stored in the owner's address. In this process the owner essentially obtains some tokens, violating the owner's limitation "cannot claim faucet tokens"

function burnFaucetTokens(uint256 amountToBurn) public onlyOwner {
require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
​
//@> should be transferring amountToBurn instead of the entire contract's token balance
_transfer(address(this), msg.sender, balanceOf(address(this)));
​
_burn(msg.sender, amountToBurn);
}

Risk

Likelihood:

  • This will happen every time the owner decides to burn tokens

Impact: High

  • This would deplete the token supply on the contract's side, rendering the faucet unusable for further user claims.
    The only solution is to mint additonal tokens but this will inflate the total token supply since the unburned tokens are just sitting in the owner's address without doing anything.

Proof of Concept

1. Owner decides to burn some amount of tokens by calling the function "burnFaucetTokens"

2. The function ends up draining all tokens from the contract, transferring them to the owner;

3. New users will not be able to claim any tokens by calling function "claimFaucetTokens"

Recommended Mitigation

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

Lead Judging Commences

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