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

FeeCollector Must Be Whitelisted in RAACToken to Prevent Fee Accounting Mismatch

mill1995

Summary

The RAACToken's tax mechanism can interfere with the FeeCollector's operations if the FeeCollector address is not whitelisted. This oversight would lead to accounting mismatches between collected and recorded fees, potentially disrupting the protocol's fee distribution system.

Vulnerability Details

The issue arises from the interaction between RAACToken's tax mechanism and FeeCollector's fee accounting when proper whitelisting is not implemented:

  1. In RAACToken, all transfers are taxed unless the sender or receiver is whitelisted:

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
@> //!! feeCollector is not excluded by default !!!
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
}

The FeeCollector records the full amount it expects to receive:

function collectFee(uint256 amount, uint8 feeType) external override nonReentrant whenNotPaused returns (bool) {
// Transfer tokens from sender
// will apply taxes if the feeCollector is not whitelisted !!!
raacToken.safeTransferFrom(msg.sender, address(this), amount);
// Records full amount in internal accounting
@> _updateCollectedFees(amount, feeType);
}

The test demonstrates this behavior:

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 {FeeCollector} from "../../contracts/core/collectors/FeeCollector.sol";
import {veRAACToken} from "../../contracts/core/tokens/veRAACToken.sol";
import {PercentageMath} from "../../contracts/libraries/math/PercentageMath.sol";
import {IFeeCollector} from "../../contracts/interfaces/core/collectors/IFeeCollector.sol";
import "forge-std/console2.sol";
​
contract FeeCollectorTest is Test {
using PercentageMath for uint256;
​
RAACToken public raacToken;
FeeCollector public feeCollector;
veRAACToken public VeRaacToken;
​
address public owner;
address public treasury;
address public repairFund;
address public minter;
address public user1;
address public user2;
​
function setUp() public {
// Setup accounts
owner = address(this);
minter = makeAddr("minter");
user1 = makeAddr("user1");
user2 = makeAddr("user2");
treasury = makeAddr("treasury");
repairFund = makeAddr("repairFund");
​
// Initial tax rates (1% swap tax, 0.5% burn tax)
uint256 initialSwapTaxRate = 100;
uint256 initialBurnTaxRate = 50;
​
// Deploy token
raacToken = new RAACToken(owner, initialSwapTaxRate, initialBurnTaxRate);
​
// Deploy veRAACToken
VeRaacToken = new veRAACToken(address(raacToken));
​
// Deploy fee collector
feeCollector = new FeeCollector(
address(raacToken),
address(VeRaacToken),
address(treasury),
address(repairFund),
owner
);
​
// Setup minter
vm.prank(owner);
raacToken.setMinter(minter);
​
// Setup fee collector
vm.prank(owner);
raacToken.setFeeCollector(address(feeCollector));
}
function test_raacTokenWrongUpdateFunction() public {
uint8 feeType = 0; // protocol fees
uint256 amount = 1000e18;
​
vm.prank(minter);
raacToken.mint(user1, amount);
​
vm.startPrank(user1);
raacToken.approve(address(feeCollector), amount);
feeCollector.collectFee(amount, feeType);
vm.stopPrank();
// get collected fees
IFeeCollector.CollectedFees memory collectedFees = feeCollector.getCollectedFees();
console2.log("recorded collectedFees", collectedFees.protocolFees);
​
uint256 balanceOfFeeCollector = raacToken.balanceOf(address(feeCollector));
​
uint256 diff = collectedFees.protocolFees - balanceOfFeeCollector;
assertGt(diff, 0);
​
console2.log("balanceOfFeeCollector", balanceOfFeeCollector);
console2.log("expectedFeeAmount", collectedFees.protocolFees);
console2.log("diff", diff);
​
// now distribute fees
vm.prank(owner);
// Fails because of wrong accounting
vm.expectRevert(IFeeCollector.InsufficientBalance.selector);
feeCollector.distributeCollectedFees();
}
}
​

Impact

If FeeCollector is not whitelisted:

  • Tokens transferred to FeeCollector would be taxed

  • FeeCollector's internal accounting would be incorrect

  • Fee distribution calculations would be inaccurate => transaction will fail

  • Protocol operations dependent on fee distribution will fail

Tools Used

  • Foundry

  • Manual Review

Recommendations

  • Add explicit whitelisting of FeeCollector in collectFee function:

function collectFee(uint256 amount, uint8 feeType) external override nonReentrant whenNotPaused returns (bool) {
// Verify FeeCollector is whitelisted
require(raacToken.isWhitelisted(address(this)),
"FeeCollector must be whitelisted");
}

  • Or exclude the feeCollector address in the _update function by default:

function _update(address from, address to, uint256 amount) internal virtual override {
// 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;
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

mill1995 Submitter
7 months ago
inallhonesty Lead Judge
7 months ago
inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!