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

Wrong Taxation in RAACToken::_update() Function

mill1995

Summary

During the RAACToken's burn function, the fee collector receives less tokens than intended due to an unintended double taxation mechanism. While the user burns the correct amount, the fee portion meant for the fee collector is incorrectly taxed again during transfer, resulting in a portion of the fees being burned instead of distributed.

Vulnerability Details

The issue occurs in the interaction between the burn() and _update() functions:

When a user burns tokens, the following sequence occurs:

  1. Correct tax amount is calculated based on burnTaxRate

  2. The main amount is properly burned

  3. _transfer() gets triggered with the taxAmount

  4. The _transfer() triggers _update which applies additional taxes to the fee portion

Example Scenario

For a burn of 1000 tokens with:

  • burnTaxRate = 50 (0.5%)

  • swapTaxRate = 100 (1%)

  • Combined tax rate = 150 (1.5%)

Current behavior:

  • User initiates burn of 1000 tokens

  • Initial burn tax calculation: 1000 * 0.5 / 100 = 5 tokens

  • 995 tokens are burned from user

  • The 5 token fee transfer triggers _update:

  • Additional 1.5% tax on 5 tokens ≈ 0.075 tokens

  • Fee collector receives ~4.975 tokens instead of 5

  • ~0.025 tokens are unexpectedly burned

Expected behavior:

  • User burns 1000 tokens

  • 995 tokens are burned

  • Fee collector receives exactly 5 tokens

  • No additional taxes on the fee portion

function burn(uint256 amount) external {
// Calculate initial tax (e.g., 0.5% of burn amount)
// 1000 * 0.5 / 100 = 5
uint256 taxAmount = amount.percentMul(burnTaxRate);
// Burn the main amount
// 1000 - 5 = 995 gets burned
_burn(msg.sender, amount - taxAmount);
if (taxAmount > 0 && feeCollector != address(0)) {
// Problem: _transfer triggers _update which applies additional taxes
// to the fee collector's portion
// _update gets called with taxAmount = 5
_transfer(msg.sender, feeCollector, taxAmount);
}
}
function _update(
address from,
address to,
uint256 amount
) internal virtual override {
// amount = 5
// baseTax = 100 + 50 = 150
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;
}
​
// 5 * 1.5 / 100 = 0.075
uint256 totalTax = amount.percentMul(baseTax);
// burnAmount = 0.075 * 50 / 150 = 0.025
uint256 burnAmount = totalTax * burnTaxRate / baseTax;
​
// send 0.075 - 0.025 = 0.05 to fee collector
super._update(from, feeCollector, totalTax - burnAmount);
// burn 0.025
super._update(from, address(0), burnAmount);
// send 5 - 0.075 = 4.925 to fee collector again
super._update(from, to, amount - totalTax);
​
// fee collector received:
// 0.05 + 4.925 = 4.975 tokens instead of 5
}

PoC

In order to run the test you need to:

  1. Run foundryup to get the latest version of Foundry

  2. Install hardhat-foundry: npm install --save-dev @nomicfoundation/hardhat-foundry

  3. Import it in your Hardhat config: require("@nomicfoundation/hardhat-foundry");

  4. Make sure you've set the BASE_RPC_URL in the .env file or comment out the forking option in the hardhat config.

  5. Run npx hardhat init-foundry

  6. There is one file in the test folder that will throw an error during compilation so rename the file in test/unit/libraries/ReserveLibraryMock.sol to => ReserveLibraryMock.sol_broken so it doesn't get compiled anymore (we don't need it anyways).

  7. Create a new folder test/foundry

  8. Paste the below code into a new test file i.e.: FoundryTest.t.sol

  9. Run the test: forge test --mc FoundryTest -vvvv

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;
​
import {Test} from "forge-std/Test.sol";
import {RAACToken} from "../../contracts/core/tokens/RAACToken.sol";
import {PercentageMath} from "../../contracts/libraries/math/PercentageMath.sol";
import "forge-std/console2.sol";
​
contract FoundryTest is Test {
using PercentageMath for uint256;
​
RAACToken public token;
​
address public owner;
address public minter;
address public user1;
address public user2;
address public feeCollector;
​
function setUp() public {
// Setup accounts
owner = makeAddr("owner");
minter = makeAddr("minter");
user1 = makeAddr("user1");
user2 = makeAddr("user2");
feeCollector = makeAddr("feeCollector");
​
// Initial tax rates (1% swap tax, 0.5% burn tax)
uint256 initialSwapTaxRate = 100;
uint256 initialBurnTaxRate = 50;
​
// Deploy token
vm.prank(owner);
token = new RAACToken(owner, initialSwapTaxRate, initialBurnTaxRate);
​
// Setup minter
vm.prank(owner);
token.setMinter(minter);
​
// Setup fee collector
vm.prank(owner);
token.setFeeCollector(feeCollector);
}
​
function testInitialState() public view {
assertEq(token.owner(), owner);
assertEq(token.minter(), minter);
assertEq(token.feeCollector(), feeCollector);
assertEq(token.swapTaxRate(), 100);
assertEq(token.burnTaxRate(), 50);
assertEq(token.name(), "RAAC Token");
assertEq(token.symbol(), "RAAC");
}
​
function testBurning() public {
uint256 amount = 1000e18;
uint256 expectedFeeAmount = amount.percentMul(token.burnTaxRate());
​
vm.prank(minter);
token.mint(user1, amount);
​
vm.prank(user1);
token.burn(amount);
​
assertEq(token.balanceOf(user1), 0);
​
uint256 balanceOfFeeCollector = token.balanceOf(feeCollector);
assertNotEq(balanceOfFeeCollector, expectedFeeAmount);
​
uint256 diff = expectedFeeAmount - balanceOfFeeCollector;
console2.log("balanceOfFeeCollector", balanceOfFeeCollector);
console2.log("expectedFeeAmount", expectedFeeAmount);
console2.log("diff", diff);
}
}

Impact

  • Fee collector receives less than the intended fee amount

  • Part of the fees are unintentionally burned

  • Inconsistency between expected and actual fee collection

Tools Used

  • Foundry

  • Manual Review

Recommendations

Exclude the feeCollector in the _update function when it's the receiver address (additional exclude it if it is the sender address if you don't want the feeCollector to pay fees during transfers):

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) ||
+ from == feeCollector ||
+ to == feeCollector
) {
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);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 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 7 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
7 months ago
inallhonesty Lead Judge 6 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.

Give us feedback!