Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

RAACToken send less tax than expected when burning

trtrth

Summary

RAACToken is a ERC20 that can take taxes on token transfers. However, in case burning tokens, the amount of tax taken is less than expected.

Vulnerability Details

The function RAACToken::burn() allows caller to burn tokens and send tax to fee collector. The burn() function will take amount.percentMul(burnTaxRate) as tax and send it to fee collector by calling _transfer(msg.sender, feeCollector, taxAmount). Here, _transfer() calls _update() with amount = taxAmount. The function _update() will handle tax logic once more. This causes the full amount taxAmount is not sent to fee collector as expected because there will be a portion of the amount is burnt

function burn(uint256 amount) external {
uint256 taxAmount = amount.percentMul(burnTaxRate);
_burn(msg.sender, amount - taxAmount);
if (taxAmount > 0 && feeCollector != address(0)) {
_transfer(msg.sender, feeCollector, taxAmount);
}
}
​
function _update(
address from,
address to,
uint256 amount
) internal virtual override {
uint256 baseTax = swapTaxRate + burnTaxRate;
// Skip tax for whitelisted addresses or when fee collector disabled
if (baseTax == 0 || from == address(0) || to == address(0) || whitelistAddress[from] || whitelistAddress[to] || feeCollector == address(0)) {
super._update(from, to, amount);
return;
}
​
// All other cases where tax is applied
@> uint256 totalTax = amount.percentMul(baseTax);
@> uint256 burnAmount = totalTax * burnTaxRate / baseTax;
super._update(from, feeCollector, totalTax - burnAmount);
@> super._update(from, address(0), burnAmount);
super._update(from, to, amount - totalTax);
}

PoC

describe("Tax Calculation", () => {
// ...
it.only("burn send less tax than expected", async function(){
// @audit Poc send less tax than expected
const transferAmount = BigInt("1000000000000000000000"); // 1000 tokens
const tax = (transferAmount * BURN_RATE) / 10000n;
​
// Mint tokens to first user (using owner who is now minter)
await raacToken.mint(users[0].address, transferAmount);
​
await raacToken.connect(users[0]).burn(transferAmount)
​
expect(await raacToken.balanceOf(feeCollector.target))
.to.equal(tax);
})

Run the test and console shows:

RAACToken
Tax Calculation
1) burn send less tax than expected
​
​
0 passing (2s)
1 failing
​
1) RAACToken
Tax Calculation
burn send less tax than expected:
​
AssertionError: expected 4975000000000000000 to equal 5000000000000000000.
+ expected - actual
​
-4975000000000000000
+5000000000000000000

Impact

  • FeeCollector receives less tax than expected

Tools Used

Manual

Recommendations

function burn(uint256 amount) external {
uint256 taxAmount = amount.percentMul(burnTaxRate);
_burn(msg.sender, amount - taxAmount);
if (taxAmount > 0 && feeCollector != address(0)) {
- _transfer(msg.sender, feeCollector, taxAmount);
+ super._update(msg.sender, feeCollector, taxAmount);
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

RAACToken::burn applies burn tax twice when transferring to feeCollector, causing excess tokens to be burned and reduced fees to be collected

This is by design, sponsor's words: Yes, burnt amount, done by whitelisted contract or not always occur the tax. The feeCollector is intended to always be whitelisted and the address(0) is included in the _transfer as a bypass of the tax amount, so upon burn->_burn->_update it would have not applied (and would also do another burn...). For this reason, to always apply such tax, the burn function include the calculation (the 2 lines that applies) and a direct transfer to feeCollector a little bit later. This is done purposefully

inallhonesty Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

RAACToken::burn applies burn tax twice when transferring to feeCollector, causing excess tokens to be burned and reduced fees to be collected

This is by design, sponsor's words: Yes, burnt amount, done by whitelisted contract or not always occur the tax. The feeCollector is intended to always be whitelisted and the address(0) is included in the _transfer as a bypass of the tax amount, so upon burn->_burn->_update it would have not applied (and would also do another burn...). For this reason, to always apply such tax, the burn function include the calculation (the 2 lines that applies) and a direct transfer to feeCollector a little bit later. This is done purposefully

Appeal created

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

RAACToken::burn applies burn tax twice when transferring to feeCollector, causing excess tokens to be burned and reduced fees to be collected

This is by design, sponsor's words: Yes, burnt amount, done by whitelisted contract or not always occur the tax. The feeCollector is intended to always be whitelisted and the address(0) is included in the _transfer as a bypass of the tax amount, so upon burn->_burn->_update it would have not applied (and would also do another burn...). For this reason, to always apply such tax, the burn function include the calculation (the 2 lines that applies) and a direct transfer to feeCollector a little bit later. This is done purposefully

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.