Submission Details
Severity:
FeeCollector FeeTypes initial values behave differently than expected
Summary
The FeeCollector::_initializeFeeTypes function contains initial values that behave differently than expected. This can lead to unexpected fee distribution.
Vulnerability Details
Initial values defined in the FeeCollector::_initializeFeeTypes will behave differently than expected.
/contracts/core/collectors/FeeCollector.sol
// Buy/Sell Swap Tax (2% total)
feeTypes[6] = FeeType({
veRAACShare: 500, // 0.5%
burnShare: 500, // 0.5%
repairShare: 1000, // 1.0%
treasuryShare: 0
});
Here the fees are expected to be distributed with ratio 1:1:2:0. But the actual distribution will be 1:1:2:16. This is because the _calculateDistribution uses 10_000 as BASIS_POINTS and all undistributed fees will be distributed to the treasury.
/contracts/core/collectors/FeeCollector.sol
function _calculateDistribution(uint256 totalFees) internal view returns (uint256[4] memory shares) {
uint256 totalCollected;
for (uint8 i = 0; i < 8; i++) {
uint256 feeAmount = _getFeeAmountByType(i);
if (feeAmount == 0) continue;
FeeType memory feeType = feeTypes[i];
totalCollected += feeAmount;
uint256 weight = (feeAmount * BASIS_POINTS) / totalFees;
# for feeTypes[6] shares[0] will be 500/10000
@> shares[0] += (weight * feeType.veRAACShare) / BASIS_POINTS;
# shares[1] will be 500/10000
@> shares[1] += (weight * feeType.burnShare) / BASIS_POINTS;
# shares[2] will be 1000/10000
@> shares[2] += (weight * feeType.repairShare) / BASIS_POINTS;
# shares[3] will be 0, so 8000/10000 are undistributed
@> shares[3] += (weight * feeType.treasuryShare) / BASIS_POINTS;
}
if (totalCollected != totalFees) revert InvalidFeeAmount();
shares[0] = (totalFees * shares[0]) / BASIS_POINTS;
shares[1] = (totalFees * shares[1]) / BASIS_POINTS;
shares[2] = (totalFees * shares[2]) / BASIS_POINTS;
shares[3] = (totalFees * shares[3]) / BASIS_POINTS;
# here undisributed fees are catched in remainder
@> uint256 remainder = totalFees - (shares[0] + shares[1] + shares[2] + shares[3]);
# and added to the treasury
@> if (remainder > 0) shares[3] += remainder;
}
This applies to feeTypes[6] and feeTypes[7].
// Buy/Sell Swap Tax (2% total)
feeTypes[6] = FeeType({
veRAACShare: 500, // 0.5%
burnShare: 500, // 0.5%
repairShare: 1000, // 1.0%
treasuryShare: 0
});
| feeTypes[6] | expected | actual |
|---|---|---|
| veRAACShare | 25% | 5% |
| burnShare | 25% | 5% |
| repairShare | 50% | 10% |
| treasuryShare | 0% | 80% |
// NFT Royalty Fees (2% total)
feeTypes[7] = FeeType({
veRAACShare: 500, // 0.5%
burnShare: 0,
repairShare: 1000, // 1.0%
treasuryShare: 500 // 0.5%
});
| feeTypes[7] | expected | actual |
|---|---|---|
| veRAACShare | 25% | 5% |
| burnShare | 0% | 0% |
| repairShare | 50% | 10% |
| treasuryShare | 25% | 85% |
PoC (foundry)
// SPDX-License-Identifier: MIT
pragma solidity 0.8.28;
import {Test, console} from "forge-std/Test.sol";
import {RAACToken} from "src/core/tokens/RAACToken.sol";
import {veRAACToken} from "src/core/tokens/veRAACToken.sol";
import {FeeCollector} from "src/core/collectors/FeeCollector.sol";
import {IFeeCollector} from "src/interfaces/core/collectors/IFeeCollector.sol";
contract FeeCollectorTestFeeTypes is Test {
FeeCollector public feeCollector;
function setUp() public {
address treasury = makeAddr("treasury");
address repairFund = makeAddr("repairFund");
RAACToken raacToken = new RAACToken(address(this), 0, 0);
raacToken.setMinter(address(this));
raacToken.mint(address(this), 100);
veRAACToken veToken = new veRAACToken(address(raacToken));
feeCollector = new FeeCollector(address(raacToken), address(veToken), treasury, repairFund, address(this));
raacToken.approve(address(feeCollector), 100);
raacToken.manageWhitelist(address(feeCollector), true);
}
function test_fee_types() public {
feeCollector.collectFee(100, 6);
vm.expectEmit(address(feeCollector));
emit IFeeCollector.FeeDistributed({
veRAACAmount: 25, // expected 25% of 100, but 5% will be distributed
burnAmount: 25, // expected 25% of 100, but 5% will be burned
repairAmount: 50, // expected 50% of 100, but 10% will be sent to repair fund
treasuryAmount: 0 // expected 0% of 100, but 80% will be sent to treasury
});
feeCollector.distributeCollectedFees();
}
}
Impact
The fees are distributed differently than expected.
Recommendations
Use BASIS_POINTS as base for shares calculation.
// Buy/Sell Swap Tax (2% total)
feeTypes[6] = FeeType({
- veRAACShare: 500, // 0.5%
+ veRAACShare: 2500, // 25%
- burnShare: 500, // 0.5%
+ burnShare: 2500, // 25%
- repairShare: 1000, // 1.0%
+ repairShare: 5000, // 50%
treasuryShare: 0
});
// NFT Royalty Fees (2% total)
feeTypes[7] = FeeType({
- veRAACShare: 500, // 0.5%
+ veRAACShare: 2500, // 25%
burnShare: 0,
- repairShare: 1000, // 1.0%
+ repairShare: 5000, // 50%
- treasuryShare: 500 // 0.5%
+ treasuryShare: 2500 // 25%
});
Updates
Lead Judging Commences
inallhonesty about 2 months ago
Submission Judgement Published Assigned finding tags:
Fee shares for fee type 6 and 7 inside FeeCollector do not total up to the expected 10000 basis points, this leads to update problems, moreover they are 10x the specifications
FeeCollector distributes too much to treasury when fee amounts are small relative to total due to precision loss in (feeAmount * BASIS_POINTS) / totalFees
inallhonesty about 2 months ago
Submission Judgement Published Assigned finding tags:
Fee shares for fee type 6 and 7 inside FeeCollector do not total up to the expected 10000 basis points, this leads to update problems, moreover they are 10x the specifications
FeeCollector distributes too much to treasury when fee amounts are small relative to total due to precision loss in (feeAmount * BASIS_POINTS) / totalFees
Prize pool breakdown
Total prize
77,280 USDC
nSLOC
3,86420 USDC / LOC
74,000
3,280
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.