Submission Details
Severity:
owner can transfer to all the balance while burnFaucetTokens
Root + Impact
Description
the burnFaucetTokens() function designed to transfer faucet balance to owner first before burning,but '''_transfer(address(this), msg.sender, balanceOf(address(this))) ''' transfer all blance not amountToBurn. lead to owner can transfer to itself all the balance while buirning only one token
// Root cause in the codebase with @> marks to highlight the relevant section
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 all balance
_burn(msg.sender, amountToBurn);
}
Risk
Impact:
owner can transfer to itself all the balance while buirning only one token
Proof of Concept
burnFaucetTokens with 1,000 tokens but owner gained 9,000 extra tokens
function testBurnFaucetTokensVulnerability() public {
vm.prank(owner);
// Record owner's balance before
uint256 ownerBalanceBefore = raiseBoxFaucet.balanceOf(owner);
uint256 totalSupplyBefore = raiseBoxFaucet.totalSupply();
// Owner calls burnFaucetTokens with 1,000 tokens
vm.prank(owner);
raiseBoxFaucet.burnFaucetTokens(1000 * 10**18);
// Check results
uint256 ownerBalanceAfter = raiseBoxFaucet.balanceOf(owner);
uint256 totalSupplyAfter = raiseBoxFaucet.totalSupply();
// VULNERABILITY PROOF:
// Owner received ALL 10,000 tokens (not just 1,000)
assertEq(
ownerBalanceAfter - ownerBalanceBefore,
10000 * 10**18,
"Owner should have received all 10,000 tokens"
);
// But only 1,000 tokens were burned from total supply
assertEq(
totalSupplyBefore - totalSupplyAfter,
1000 * 10**18,
"Only 1,000 tokens should have been burned"
);
// This means owner gained 9,000 extra tokens
uint256 extraTokens = (ownerBalanceAfter - ownerBalanceBefore) - (totalSupplyBefore - totalSupplyAfter);
assertEq(extraTokens, 9000 * 10**18, "Owner gained 9,000 extra tokens");
// Contract balance should be 0
assertEq(
raiseBoxFaucet.balanceOf(address(raiseBoxFaucet)),
0,
"Contract balance should be 0"
);
}
Traces:
[59494] TestRaiseBoxFaucet::testBurnFaucetTokensVulnerability()
├─ [0] VM::prank(TestRaiseBoxFaucet: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return]
├─ [2917] RaiseBoxFaucet::balanceOf(TestRaiseBoxFaucet: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
│ └─ ← [Return] 0
├─ [2545] RaiseBoxFaucet::totalSupply() [staticcall]
│ └─ ← [Return] 1000000000000000000000000000 [1e27]
├─ [0] VM::prank(TestRaiseBoxFaucet: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return]
├─ [36498] RaiseBoxFaucet::burnFaucetTokens(1000000000000000000000 [1e21])
│ ├─ emit Transfer(from: RaiseBoxFaucet: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], to: TestRaiseBoxFaucet: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], value: 1000000000000000000000000000 [1e27])
│ ├─ emit Transfer(from: TestRaiseBoxFaucet: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], to: 0x0000000000000000000000000000000000000000, value: 1000000000000000000000 [1e21])
│ └─ ← [Stop]
├─ [917] RaiseBoxFaucet::balanceOf(TestRaiseBoxFaucet: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
│ └─ ← [Return] 999999000000000000000000000 [9.999e26]
├─ [545] RaiseBoxFaucet::totalSupply() [staticcall]
│ └─ ← [Return] 999999000000000000000000000 [9.999e26]
├─ [0] VM::assertEq(999999000000000000000000000 [9.999e26], 10000000000000000000000 [1e22], "Owner should have received all 10,000 tokens") [staticcall]
│ └─ ← [Revert] Owner should have received all 10,000 tokens: 999999000000000000000000000 != 10000000000000000000000
└─ ← [Revert] Owner should have received all 10,000 tokens: 999999000000000000000000000 != 10000000000000000000000
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 13.18ms (2.60ms CPU time)
Ran 1 test suite in 319.93ms (13.18ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/RaiseBoxFaucet.t.sol:TestRaiseBoxFaucet
[FAIL: Owner should have received all 10,000 tokens: 999999000000000000000000000 != 10000000000000000000000] testBurnFaucetTokensVulnerability() (gas: 59494)
Encountered a total of 1 failing tests, 0 tests succeeded
Recommended Mitigation
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)));
+ require(amountToBurn <= balanceOf(address(this)), "Faucet Token Balance: Insufficient");
+ _transfer(address(this), msg.sender, amountToBurn));
_burn(msg.sender, amountToBurn);
}
Updates
Lead Judging Commences
inallhonesty 8 days ago
Submission Judgement Published Assigned finding tags:
Unnecessary and convoluted logic in burnFaucetTokens
Rewards breakdown
nSLOC
157This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.