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

The `RaiseBoxFaucet::burnFaucetTokens` transfers the entire balance of the contract to the owner instead of the `amountToBurn`, draining and potentially causing a denial of service (DoS).

alexscherbatyuk

Root + Impact

Description

The _transfer function of the contract transfers the entire balance of the contract balanceOf(address(this)) to the owner instead of the amountToBurn, and then burns the amountToBurn from the owner.

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:

  • Anytime the owner performs a burn of any amount specified in the burnFaucetTokens(uint256 amountToBurn) call, it automatically drains all tokens available for distribution to the owner's account.

Impact:

  • Intended functionality for normal administrative routine functions drains the faucet tokens, breaks the main protocol's functionality, and causes a denial of service for users.

Proof of Concept

Add the following code snippet to the RaiseBoxFaucet.t.sol test file.

function testBurnEntireSupplyFaucetTokens() public {
vm.prank(owner);
uint256 balanceOfContract = raiseBoxFaucet.balanceOf(address(raiseBoxFaucet));
uint256 amountToBurn = 1000e18;
raiseBoxFaucet.burnFaucetTokens(amountToBurn);
uint256 balanceOfContractAfterBurn = raiseBoxFaucet.balanceOf(address(raiseBoxFaucet));
​
console.log("Balance of contract after burn: ", raiseBoxFaucet.balanceOf(address(raiseBoxFaucet)));
assertNotEq(
balanceOfContract,
balanceOfContractAfterBurn + amountToBurn,
"Balance of contract after burn + amount to burn is not equal to balance of contract before burn"
);
assertEq(raiseBoxFaucet.balanceOf(address(raiseBoxFaucet)), 0, "Balance of contract after burn is 0");
}

Recommended Mitigation

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

Lead Judging Commences

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