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

Due to mishandling of tokens, the owner can run burnFaucetTokens() to transfer tokens to themself and make the contract stop working

farnad

Root + Impact

Description

  • Even though mentioned in the comments of the program, logically it is wrong to transfer total balance of the contract, instead of amountToBurn to the owner.

  • It makes the contract non-functional due to the lack of faucet tokens to give to the users.

  • It also indirectly breaks one of the invariants of the application, which is "Owner cannot claim faucet tokens". It has the effect of allowing the owner to transfer some or all of the faucet tokens to their own account.

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: Medium

  • This will happen only when the owner calls the RaiseBoxFaucet::burnFaucetTokens function.

Impact: High

  • Depending on the value of the amountToBurn argument, the owner can intentionally/uintentionally transfer some or all of the faucet tokens to their own account, and burn the rest. It also breaks the invariant of the application.


  • Another effect is making the contract run out of the faucet tokens and stop working (RaiseBoxFaucet::claimFaucetTokens function calls will revert) until the owner mints more tokens.

Proof of Concept

Please copy and paste the following code to the test file, and run it.

function testOwnerCanDrainFaucetBalanceUsingBurnFunction() public {
// Arrange & Act
vm.prank(owner);
raiseBoxFaucet.burnFaucetTokens(0); // Burn no tokens but transfer all to the owner account
// Assert
assert(raiseBoxFaucet.getFaucetTotalSupply() == 0);
assert(raiseBoxFaucet.getBalance(owner) == INITIAL_SUPPLY_MINTED);
vm.prank(user1);
vm.expectRevert();
raiseBoxFaucet.claimFaucetTokens(); // No more tokens to claim
}

Recommended Mitigation

This issue can be fixed by replacing balanceOf(address(this)) with amountToBurn in the _transfer function call.

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