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

Not all tokens are burnt when using `burnFaucetTokens()` function

k3ysk1lls

Description

The goal of the RaiseBoxFaucet::burnFaucetTokens() function is to make it possible to burn tokens by the owner of the contract. During this process, if the owner wants to burn less than totalSupply, all the tokens will be transferred to the owner's address and only the specified number of tokens will be burnt, resulting in the rest of the tokens remaining untouched in the owner's address.

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 likelihood is high since whenever the RaiseBoxFaucet::burnFaucetTokens() function is triggered by the owner of the contract, the transfer of all tokens in supply happens, resulting in the faucet's total supply resetting to zero.

Impact:

Since it can be triggered by the owner (a trusted person), the impact is low.

Proof of Concept

Paste the followinf code into the test file and run forge test --mt testBurnTokens:

function testBurnTokens() public {
uint256 totalSupplyBefore = raiseBoxFaucet.getFaucetTotalSupply();
uint256 amountToBurn = totalSupplyBefore / 2; //less than totalSupplyBefore
uint256 ownerTokenBalanceBefore = raiseBoxFaucet.getBalance(owner);
​
assertEq(totalSupplyBefore, raiseBoxFaucet.getBalance(raiseBoxFaucetContractAddress));
​
vm.prank(owner);
raiseBoxFaucet.burnFaucetTokens(amountToBurn);
​
uint256 totalSupplyAfter = raiseBoxFaucet.getFaucetTotalSupply();
uint256 ownerTokenBalanceAfter = raiseBoxFaucet.getBalance(owner);
​
assertEq(totalSupplyAfter, 0);
assertEq(ownerTokenBalanceAfter, ownerTokenBalanceBefore + totalSupplyBefore - amountToBurn);
}
​

Recommended Mitigation

Make the following changes to burnFaucetTokens function:

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 6 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.