Core Contracts

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

RAACToken::burn triggers tax collection when submitting burn tax

Summary

RAACToken contract is an implementation of ERC20, so it inherits the functions from the ERC20 standard. Most of the functions are overriden to make the customization as intended or as per the design suitable for this protocol. However, the issue lies in the burn operation. So as per the natspec, docs and even as per the code there is a taxation mechanism for burn and swap mechanism.
When a user calls the burn function, the calculated taxAmount is transferred to the feeCollector. This transfer triggers the _update function, reapplying the swap and burn taxes on the taxAmount. As a result, the taxAmount is taxed multiple times, leading to higher effective taxation than intended.

Vulnerability Details

So as per the burn function, the tax amount is deducted from the amount specified. The taxAmount is then transferred to feeCollector, however things takes a turn around when _transfer calls the underlying _update

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

Although_update has the rule to not take tax when burn() is called. However, while transferring the burn tax to feeCollector, it submits tax on tax. Sounds funny, but this is actually what happens. You can check the clause below the if.

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

Impact

  • More amount of tax, is deducted and also uneven tax is submitted to different actors - feeCollector, burn, and to fee collector remaining tax which makes the state update of supply insufficient and wrong.

Tools Used

Manual Review

Recommendations

  • Make the transfer to different actors specifically in the functions rather than in _update.

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