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

The burning token process drains protocol reserves tokens

duma999

The burning token process drains protocol reserves tokens

Description

The function BurnFaucetTokens transfers all tokens to the owner when he/she wants to burn any amount of tokens.

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);
}

The reserves of the protocol are drained (unintentionally) causing the claimFaucetTokens function to stop working as there is insufficient balance.

Risk

Likelyhood(High) : It will append every time the owner burns tokens.
Impact(High) : The core functionality of the protocol will be blocked as there is insufficient token balance in it.

Proof of Concept

Add this test to RaiseBoxFaucet.t.sol :

function testBurnFunction() public {
vm.prank(user1);
raiseBoxFaucet.claimFaucetTokens();
​
vm.prank(user2);
raiseBoxFaucet.claimFaucetTokens();
​
// Owner burn 10 token, not big deal.
vm.prank(owner);
raiseBoxFaucet.burnFaucetTokens(10);
​
​
// Fail: RaiseBoxFaucet_InsufficientContractBalance because of a small burn append before.
vm.expectRevert();
vm.prank(user3);
raiseBoxFaucet.claimFaucetTokens();
}

Recommended Mitigation

Remove the requirement that the tokens should be transferred to the owner, and burn the right amount directly.

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