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

Burn function always transfer all tokens to the owner and block claims

mkozhuharov

Root + Impact / Burn function always transfer all tokens to the owner and block claims

Description

  • Under normal conditions, when the protocol owner wants to burn faucet tokens, the function should transfer the specified amount to the owner and then burn exactly that amount.

  • In RaiseBoxFaucet.sol::burnFaucetTokens, instead of transferring the requested amountToBurn, the entire Faucet Token balance is transferred to the owner. The function then burns only the amountToBurn, leaving the remaining tokens in the owner’s balance.

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 issue occurs each time the owner invokes the RaiseBoxFaucet.sol::burnFaucetTokens function.

Impact:

  • The protocol will lose its entire Faucet Token balance, making it unable to provide tokens for users to claim, thus making the system non-operational.

  • This vulnerability also violates a core invariant: the owner should not be able to claim faucet tokens. However, due to this flaw, the owner effectively gains control over the Faucet Token supply by receiving the entire balance.

Proof of Concept

Add the following test to RaiseBoxFaucet.t.sol to reproduce the issue:

function test_audit_burnFunctionAlwaysTransferAllTokensToTheOwnerAndBlockClaims()
public
{
assertEq(raiseBoxFaucet.getFaucetTotalSupply(), INITIAL_SUPPLY_MINTED);
​
vm.prank(owner);
raiseBoxFaucet.burnFaucetTokens(0);
​
assertEq(raiseBoxFaucet.getFaucetTotalSupply(), 0);
assertEq(raiseBoxFaucet.getBalance(owner), INITIAL_SUPPLY_MINTED);
}

Recommended Mitigation

When performing the transfer, use the provided amountToBurn parameter instead of the full balance.

- _transfer(address(this), msg.sender, balanceOf(address(this)));
+ _transfer(address(this), msg.sender, amountToBurn);
Updates

Lead Judging Commences

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