The totals() function is responsible for calculating the total shares for RSR and RToken distribution. It's a critical component for ensuring proper revenue distribution and maintaining system invariants.
The issue is in the DAO fee calculation part of the function. The current implementation can lead to an inflation of the rsrTotal, potentially breaking system invariants and causing unexpected behavior in revenue distribution.
Details
The function calculates base totals correctly.
When a DAO fee is present, it adds an additional amount to rsrTotal.
This addition can cause the total distribution to exceed 100% (or MAX_DISTRIBUTION).
The function doesn't adjust other distributions to accommodate the DAO fee, leading to an inflated total.
Violation of system invariants: The total distribution may exceed 100%.
Incorrect revenue distribution: Other recipients may receive less than their intended share.
Potential for system instability or unexpected behavior in other components that rely on these totals.
Scenario
Suppose the initial distribution is:
RToken: 50%
RSR: 50%
DAO Fee: 10%
The totals() function will return:
RToken: 50%
RSR: 55.55% (50% + 5.55% DAO fee)
Total distribution: 105.55%, which exceeds 100%.
Fix
To address this issue, the function should be modified to incorporate the DAO fee without inflating the total distribution. Here's a potential fix:
Calculate the DAO fee separately.
Adjust the RSR and RToken totals proportionally to accommodate the DAO fee.
Ensure the final total (including the DAO fee) equals MAX_DISTRIBUTION.
Example implementation:
function totals() public view returns (RevenueTotals memory revTotals) {
// Calculate base totals as before
...
DAOFeeRegistry daoFeeRegistry = main.daoFeeRegistry();
if (address(daoFeeRegistry) != address(0)) {
(address feeRecipient, uint256 feeNumerator, uint256 feeDenominator) = main.daoFeeRegistry().getFeeDetails(address(rToken));
if (feeRecipient != address(0) && feeNumerator != 0) {
uint256 totalBeforeFee = revTotals.rTokenTotal + revTotals.rsrTotal;
uint256 daoFee = (totalBeforeFee * feeNumerator) / feeDenominator;
// Adjust RSR and RToken totals proportionally
revTotals.rsrTotal = uint24((uint256(revTotals.rsrTotal) * (totalBeforeFee - daoFee)) / totalBeforeFee);
revTotals.rTokenTotal = uint24((uint256(revTotals.rTokenTotal) * (totalBeforeFee - daoFee)) / totalBeforeFee);
// Add DAO fee to RSR total
revTotals.rsrTotal += uint24(daoFee);
}
}
require(revTotals.rsrTotal + revTotals.rTokenTotal == MAX_DISTRIBUTION, "Invalid total distribution");
}
This fix ensures that the total distribution always equals MAX_DISTRIBUTION, maintaining system invariants and providing a more accurate representation of revenue shares.
Lines of code
https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Distributor.sol#L204
Vulnerability details
Vulnerability Details
The
totals()
function is responsible for calculating the total shares for RSR and RToken distribution. It's a critical component for ensuring proper revenue distribution and maintaining system invariants.The issue is in the DAO fee calculation part of the function. The current implementation can lead to an inflation of the
rsrTotal
, potentially breaking system invariants and causing unexpected behavior in revenue distribution.Details
rsrTotal
.MAX_DISTRIBUTION
).Code Snippet
Impact
Scenario
Suppose the initial distribution is:
The
totals()
function will return:Total distribution: 105.55%, which exceeds 100%.
Fix
To address this issue, the function should be modified to incorporate the DAO fee without inflating the total distribution. Here's a potential fix:
MAX_DISTRIBUTION
.Example implementation:
This fix ensures that the total distribution always equals
MAX_DISTRIBUTION
, maintaining system invariants and providing a more accurate representation of revenue shares.Assessed type
Context