Core Contracts

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

Transfer fees for RAACToken not correctly sent to the FeeCollector

Summary

Whenever there is a transfer or burn of RAACTokens we have a fee that needs to be paid, the logic is _update function of the RAACToken contract. This fee should then be transfered to theFeeCollector where we can distribute it along with other fees as rewards to veRAAC token holders, distribute some amount to tresury and etc. The main issue is that we directly update the balance of the FeeCollectorwith the fee amount instead of calling the collectFee function that accuretly reflects collected fees.

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;
}
// 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);
// }
// }
// All other cases where tax is applied
uint256 totalTax = amount.percentMul(baseTax);
uint256 burnAmount = totalTax * burnTaxRate / baseTax;
//@Audit we don't use the feeCollector "collectFees" mechanism so distribution is missing this fees when doing distributions
super._update(from, feeCollector, totalTax - burnAmount);
super._update(from, address(0), burnAmount);
super._update(from, to, amount - totalTax);
}

Vulnerability Details

Due to incorrect approch to update the balance of FeeCollector with transfer fees from RAACToken tokens we do not distribute the accumulated amount when distribution of fees is called on the FeeCollector. Instead of just updating the balance of the FeeCollector address with the fee amount for the RAACTokens transfer, we should call something similar to this feeCollector.collectFee(from, totalTax - burnAmount, 0) which accuratly updates the amount of fees collected. We coul either pass as a second argument "0" or "4" depending if we consider this to be a protocolFees or a mintRedeemFees. NOTE THAT THIS WILL NOT WORK CORRECLTY CURRENTLY AS THE from value is not supported by the collectFee function, it uses the msg.sender to collect the provided amount, in the current logic the RAACToken contract which is not correct for this scenario.

Impact

Transfer and burn fees are not reflected when we do a distribution of fees in the FeeCollector contract. This in turn would mean that veRAAC token holders will receive less rewards than they should have when the distribution is executed.

Tools Used

Manual review

Recommendations

Instead of updating directly the balance of the FeeCollector with the fee amount of the RAACToken transfer/burn, create a new function/modify the collectFees function of the FeeCollector so that we can pass an account from which to transfer the RAACTokens and reflect them for future distributions.

function collectFee(uint256 amount, uint8 feeType) external override nonReentrant whenNotPaused returns (bool) {
if (amount == 0 || amount > MAX_FEE_AMOUNT) revert InvalidFeeAmount();
if (feeType > 7) revert InvalidFeeType();
// Transfer tokens from sender
//@Audit This uses message msg.sender currently which is not the correct scenario for this specific use case
//as msg.sender will be the RAACToken contact instead of the user that is initiating the transfer/burn
raacToken.safeTransferFrom(msg.sender, address(this), amount);
// Update collected fees
_updateCollectedFees(amount, feeType);
emit FeeCollected(feeType, amount);
return true;
}
Updates

Lead Judging Commences

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

RAACToken::burn sends tax directly to FeeCollector without using collectFee(), causing tokens to bypass accounting and remain undistributed. `collectFee` is not used anywhere.

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

RAACToken::burn sends tax directly to FeeCollector without using collectFee(), causing tokens to bypass accounting and remain undistributed. `collectFee` is not used anywhere.

Support

FAQs

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

Give us feedback!