Core Contracts

Summary

The FeeCollector contract exhibits an inconsistency between the initialization of fee types in _initializeFeeTypes and the validation logic in updateFeeType. Specifically, feeTypes[6] (Swap Tax) and feeTypes[7] (NFT Royalties) are initialized with a total share percentage of 20% (2000 basis points), while updateFeeType enforcing a strict requirement that the sum of all shares for any fee type must equal 100% (BASIS_POINTS = 10000). This inconsistency creates confusion, limits the flexibility of the fee system, and could potentially lead to unexpected behavior or vulnerabilities if the distribution logic is modified in the future.

Vulnerability Details

_initializeFeeTypes()

The core issue lies in the discrepancy between these two functions:

• _initializeFeeTypes (internal): This function initializes the default fee types and their corresponding share distributions. The relevant code snippets are:

  // Buy/Sell Swap Tax (2% total)

  feeTypes[6] = FeeType({

  veRAACShare: 500, // 0.5%

  burnShare: 500, // 0.5%

  repairShare: 1000, // 1.0%

  treasuryShare: 0

  });

  ​

  // NFT Royalty Fees (2% total)

  feeTypes[7] = FeeType({

  veRAACShare: 500, // 0.5%

  burnShare: 0,

  repairShare: 1000, // 1.0%

  treasuryShare: 500 // 0.5%

  });

  ​

  ​

  ​

  
  The sum of shares for feeTypes[6] is 500 + 500 + 1000 + 0 = 2000.
  The sum of shares for feeTypes[7] is 500 + 0 + 1000 + 500 = 2000.
  These sums equal 2000 basis points (20%), not BASIS_POINTS (10000, or 100%).

  updateFeeType(https://github.com/Cyfrin/2025-02-raac/blob/89ccb062e2b175374d40d824263a4c0b601bcb7f/contracts/core/collectors/FeeCollector.sol#L220)

• updateFeeType (external): This function allows the FEE_MANAGER_ROLE to update the parameters of a specific fee type. The relevant code snippet is:

  function updateFeeType(uint8 feeType, FeeType calldata newFee) external override {

  // ... (Role and type checks)

  ​

  // Validate fee shares total to 100%

  if (newFee.veRAACShare + newFee.burnShare + newFee.repairShare + newFee.treasuryShare != BASIS_POINTS) {

  revert InvalidDistributionParams();

  }

  ​

  // ...

  }

This function explicitly requires that the sum of the shares in newFee equals BASIS_POINTS (10000).
The inconsistency is that _initializeFeeTypes sets up feeTypes[6] and feeTypes[7] in a way that would immediately cause updateFeeType to revert if it were called with the same values. This creates a confusing and potentially problematic situation.

Impact

The FEE_MANAGER_ROLE is prevented from setting feeTypes[6] or feeTypes[7] to any configuration where the total shares do not equal 100%. This restricts the ability to adjust the fee distribution in the future. For example, it would be impossible to disable the treasuryShare for these fee types without also modifying other shares to compensate.

Tools Used

Manual Review

Recommendations

We can add a condition to the updateFeeType function to specific updates for feeTypes[6] or feeTypes[7].