Submission Details
Severity:
Lack of Maximum Fee Check in updateFeeType May Lead to Excessive Fees Resulting in Unexpected Fund Loss
Summary
The `updateFeeType` function in the `FeeCollector.sol` contract does not enforce a check to ensure that the total fee amount does not exceed `MAX_FEE_AMOUNT`. This could allow setting fees that exceed the intended maximum, potentially leading to excessive fees being charged, which could harm users.
Vulnerability Details
The updateFeeType function is responsible for updating the fee structure for different fee types. It currently performs a validation check to ensure that the sum of veRAACShare, burnShare, repairShare, and treasuryShare equals BASIS_POINTS. However, it does not check whether the total fee exceeds the MAX_FEE_AMOUNT defined in the contract:
```solidity
function updateFeeType(uint8 feeType, FeeType calldata newFee) external override {
if (!hasRole(FEE_MANAGER_ROLE, msg.sender)) revert UnauthorizedCaller();
// Validate fee shares total to 100%
if (newFee.veRAACShare + newFee.burnShare + newFee.repairShare + newFee.treasuryShare != BASIS_POINTS) {
revert InvalidDistributionParams();
}
feeTypes[feeType] = newFee;
emit FeeTypeUpdated(feeType, newFee);
}
```
Given that `MAX_FEE_AMOUNT` is set as:
```solidity
uint256 public constant MAX_FEE_AMOUNT = 1_000_000e18;
```
The function does not enforce a check ensuring that newFee does not exceed this maximum limit. This omission could lead to excessive fees being set, which could negatively impact users.
### Proof of Concept
A simple scenario demonstrating the issue:
A user calls updateFeeType with an excessively high fee structure.
The function accepts the fee structure since there is no validation against `MAX_FEE_AMOUNT`.
When a transaction requiring fee deductions occurs, users will be overcharged.
Impact
If an excessive fee is set due to the lack of a `MAX_FEE_AMOUNT` check, users may be unfairly charged, leading to significant losses. This could damage the protocol’s reputation and discourage user participation. The issue is particularly concerning if fee updates are performed by an entity with the FEE_MANAGER_ROLE, as they could unintentionally (or maliciously) set an unreasonably high fee.
This issue is rated High severity because it directly affects fund distribution and could result in economic losses for users.
Tools Used
Manual Review and Foundry
Recommendations
To prevent excessive fees, add a validation check in updateFeeType to ensure that the total fee does not exceed MAX_FEE_AMOUNT. The modified function should include:
```solidity
function updateFeeType(uint8 feeType, FeeType calldata newFee) external override {
if (!hasRole(FEE_MANAGER_ROLE, msg.sender)) revert UnauthorizedCaller();
// Validate fee shares total to 100%
if (newFee.veRAACShare + newFee.burnShare + newFee.repairShare + newFee.treasuryShare != BASIS_POINTS) {
revert InvalidDistributionParams();
}
// Ensure the total fee does not exceed MAX_FEE_AMOUNT
if (newFee.veRAACShare + newFee.burnShare + newFee.repairShare + newFee.treasuryShare > MAX_FEE_AMOUNT) {
revert ExceedsMaximumFee();
}
feeTypes[feeType] = newFee;
emit FeeTypeUpdated(feeType, newFee);
}
```
This fix ensures that the sum of all fee components does not exceed the defined maximum, preventing excessive fees from being set.
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.