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

Incorrect burn tax collection on each RAACToken burn()

t0x1c

Description

Inside RAACToken.sol, the overridden _update() function is called after each burn() and transfer() call. While no additional calculation happens in case of burn() as to == address(0), additional tax calculation does happen in transfer():

File: contracts/core/tokens/RAACToken.sol
​
185: function _update(
186: address from,
187: address to,
188: uint256 amount
189: ) internal virtual override {
190: uint256 baseTax = swapTaxRate + burnTaxRate;
191: // Skip tax for whitelisted addresses or when fee collector disabled
192: if (baseTax == 0 || from == address(0) || to == address(0) || whitelistAddress[from] || whitelistAddress[to] || feeCollector == address(0)) {
193: super._update(from, to, amount);
194: return;
195: }
196:
197: // All other cases where tax is applied
198: uint256 totalTax = amount.percentMul(baseTax);
199: uint256 burnAmount = totalTax * burnTaxRate / baseTax;
200:
201: super._update(from, feeCollector, totalTax - burnAmount);
202:@---> super._update(from, address(0), burnAmount);
203: super._update(from, to, amount - totalTax);
204: }

This causes reduced amount of fee to be collected each time. Let's consider the burn() call:

File: contracts/core/tokens/RAACToken.sol
​
80: function burn(uint256 amount) external {
81: uint256 taxAmount = amount.percentMul(burnTaxRate);
82:@---> _burn(msg.sender, amount - taxAmount);
83: if (taxAmount > 0 && feeCollector != address(0)) {
84: _transfer(msg.sender, feeCollector, taxAmount);
85: }
86: }

Imagine:

  1. amount = 1_000_000 and burnTaxRate = 0.5%

  2. Thus taxAmount = 0.5% * 1_000_000 = 5_000

  3. L82 burns 1_000_000 - 5_000 = 995_000

  4. L84 calls ERC20's _transfer(msg.sender, feeCollector, 5000) which internally calls _update() and control goes to L185, our overridden _update() implementation

  5. amount here is 5000. Also, baseTax = swapTaxRate + burnTaxRate = 1% + 0.5% = 1.5%

  6. On L198 totalTax = 1.5% of 5000 = 75

  7. On L199 burnAmount = 75 * 0.5% / 1.5% = 25

  8. On L201 feeCollector gets totalTax - burnAmount = 75 - 25 = 50

  9. On L202 address(0) gets burnAmount = 25 i.e. it is burned!

  10. On L203 feeCollector (same as to) gets amount - totalTax = 5000 - 75 = 4925

As we can see the burnTaxRate was erroneously applied on the initial 1_000_000 twice. Instead of burning 995_000 RAAC tokens and passing on the rest as fees, the code burns 995_000 + 25 = 995_025 tokens. And the feeCollector collects a reduced figure of 4975 as tax instead of 5000.

Note that to fix this, we can NOT change L202 to:

- 202: super._update(from, address(0), burnAmount);
+ 202: super._update(from, feeCollector, burnAmount);

because changing this will mess up the normal transfer() calls and the current mechanics is supposed to burn the burnAmount (calculated based on burnTaxRate) by sending it to address(0).

Impact

As a result of this:

  • The actual circulating supply will be lower than intended

  • This amplifies the deflationary effect intended by the protocol's tokenomics

  • The feeCollector receives less fee each time.

Note that burn() is often called by the protocol. Inside FeeCollector.sol, the execution flow is distributeCollectedFees() --> _processDistributions() and _processDistributions() has:

L421: if (shares[1] > 0) raacToken.burn(shares[1]);

Proof of Concept

Add this as a file named RAACburn.test.js under test/unit/core/pools/LendingPool/ and run with npx hardhat test test/unit/core/pools/LendingPool/RAACburn.test.js:

import { expect } from "chai";
import hre from "hardhat";
const { ethers } = hre;
​
describe("RAACToken", function () {
let raacToken;
let owner;
let user1;
let feeCollector;
​
beforeEach(async function () {
[owner, user1, feeCollector] = await ethers.getSigners();
​
const RAACToken = await ethers.getContractFactory("RAACToken");
// Constructor params: initialOwner, initialSwapTaxRate (100 = 1%), initialBurnTaxRate (50 = 0.5%)
raacToken = await RAACToken.deploy(owner.address, 100, 50);
// Set fee collector
await raacToken.setFeeCollector(feeCollector.address);
// Mint some tokens to user1 for testing
await raacToken.setMinter(owner.address);
await raacToken.mint(user1.address, ethers.parseEther("1000000")); // 1M tokens
});
​
describe("Burn Tax Calculation", function () {
it("should demonstrate double burn tax application issue", async function () {
const initialBalance = await raacToken.balanceOf(user1.address);
const initialFeeCollectorBalance = await raacToken.balanceOf(feeCollector.address);
const initialTotalSupply = await raacToken.totalSupply();
​
console.log("\nInitial state:");
console.log("Initial Total Supply:", ethers.formatEther(initialTotalSupply));
console.log("Initial User Balance:", ethers.formatEther(initialBalance));
console.log("Initial Fee Collector Balance:", ethers.formatEther(initialFeeCollectorBalance));
​
// Amount to burn: 1,000,000 tokens
const burnAmount = ethers.parseEther("1000000");
await raacToken.connect(user1).burn(burnAmount);
​
const finalBalance = await raacToken.balanceOf(user1.address);
const finalFeeCollectorBalance = await raacToken.balanceOf(feeCollector.address);
const finalTotalSupply = await raacToken.totalSupply();
const totalBurned = initialTotalSupply - finalTotalSupply;
​
console.log("\nFinal state:");
console.log("Final Total Supply:", ethers.formatEther(finalTotalSupply));
console.log("Final User Balance:", ethers.formatEther(finalBalance));
console.log("Final Fee Collector Balance:", ethers.formatEther(finalFeeCollectorBalance));
console.log("Total Burned:", ethers.formatEther(totalBurned));
​
// User's balance should be 0
expect(finalBalance).to.equal(0);
​
// Total burned would be 995,000 + 25 = 995,025
const expectedBurn = ethers.parseEther("995025");
expect(totalBurned).to.equal(expectedBurn);
​
// Fee collector would get 5,000 - 25 = 4,975 tokens
const expectedFeeCollectorBalance = ethers.parseEther("4975");
expect(finalFeeCollectorBalance).to.equal(expectedFeeCollectorBalance);
​
});
});
});

Output:

RAACToken
Burn Tax Calculation
​
Initial state:
Initial Total Supply: 1000000.0
Initial User Balance: 1000000.0
Initial Fee Collector Balance: 0.0
​
Final state:
Final Total Supply: 4975.0
Final User Balance: 0.0
Final Fee Collector Balance: 4975.0
Total Burned: 995025.0
✔ should demonstrate double burn tax application issue (46ms)
​
​
1 passing (6s)

Mitigation

One way would be to temporarily whitelist the feeCollector address inside burn() so that the custom calculation inside _update() is entirely skipped:

function burn(uint256 amount) external {
uint256 taxAmount = amount.percentMul(burnTaxRate);
_burn(msg.sender, amount - taxAmount);
if (taxAmount > 0 && feeCollector != address(0)) {
+ bool isAlreadyWhitelisted = whitelistAddress[feeCollector];
+ if (!isAlreadyWhitelisted)
+ whitelistAddress[feeCollector] = true;
_transfer(msg.sender, feeCollector, taxAmount);
+ // go back to initial whitelisted state
+ if (!isAlreadyWhitelisted)
+ whitelistAddress[feeCollector] = false;
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 3 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 3 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 2 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.