Raisebox Faucet

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

When owner calls `RaiseBoxFaucet::burnFaucetTokens` the contract balance is transfered to owner, conflicting with docs where owner should not be able to claim faucet tokens

Root + Impact

Description

  • Owner can call RaiseBoxFaucet::burnFaucetTokens function to burn a specified amount of RaiseBoxtokens.

  • When owner calls this function the full balance of the contract is transfered to the owner no matter how much is 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:

  • The contract gets depleted every time the burnFaucetTokensfunction gets called so likelyhood is 100%.

Impact:

  • The docs specifies that owner can not claim faucet tokens but the owner effectivelly can claim tokens through this function by inputting a small amount to burn and then keep the whole balance of the RaiseBoxFaucetcontract.


Documentaion:

LIMITATIONS:

  • cannot claimfaucet tokens

Proof of Concept

Flow

  1. Owner calls burnFaucetTokensfunction to burn 100 tokens

  2. The function transfer the full balance of the RaiseBoxFaucetcontract to the owner depleting the faucet

  3. The 100 tokens get burned from the owner leaving the owner with the contracts full balance minus the 100 tokens

Add this test to the test suite:

function testBurnFaucetTokens() public {
// The contract balance before burn
uint256 initialBalance = raiseBox.balanceOf(address(raiseBox));
console.log("Initial contract balance:", initialBalance);
// Burn 100 tokens
uint256 burnAmount = 100e18;
// Owner is this test contract
raiseBox.burnFaucetTokens(burnAmount);
// The contract balance after burn is 0
uint256 finalBalance = raiseBox.balanceOf(address(raiseBox));
console.log("Final contract balance:", finalBalance);
// Owner balance after burn
uint256 ownerBalance = raiseBox.balanceOf(address(this));
console.log("Owner balance:", ownerBalance);
// After burning, the contract should have 0 balance
assertEq(raiseBox.balanceOf(address(raiseBox)), 0);
// The owner's balance should be the initial contract balance minus the burned amount
assertEq(raiseBox.balanceOf(address(this)), initialBalance - burnAmount);
}

Output:

Logs:
Initial contract balance: 1000000000000000000000000000
Final contract balance: 0
Owner balance: 999999900000000000000000000

Recommended Mitigation

Burn the tokens directly from the RaiseBoxFaucetcontract:

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.