Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/Distributor.sol#L131

Vulnerability details

Vulnerability Details

The distribute function is responsible for distributing RSR or RToken to various destinations based on predefined shares. It's a critical function in the revenue distribution mechanism of the system.

The bug lies in the calculation and distribution of tokens. The function calculates tokensPerShare by dividing the total amount by the total shares, but it uses integer division, which can lead to rounding errors. These errors accumulate and result in undistributed tokens.

Code Snippet

uint256 tokensPerShare;
uint256 totalShares;
{
    RevenueTotals memory revTotals = totals();
    totalShares = isRSR ? revTotals.rsrTotal : revTotals.rTokenTotal;
    if (totalShares != 0) tokensPerShare = amount / totalShares;
    require(tokensPerShare != 0, "nothing to distribute");
}

// ... later in the function:
uint256 transferAmt = tokensPerShare * numberOfShares;

Impact

Loss of Funds: Due to rounding errors, a portion of tokens may remain undistributed. These tokens effectively become locked in the contract.
Unfair Distribution: Destinations with smaller shares are more likely to be adversely affected by rounding errors, leading to an unfair distribution over time.
Inconsistency with Total Amount: The sum of distributed tokens may be less than the intended distribution amount.

Scenario

Suppose there are 100 tokens to distribute among 3 destinations with shares of 50%, 30%, and 20%.

tokensPerShare = 100 / 100 = 1
Destination 1 receives: 1 * 50 = 50 tokens
Destination 2 receives: 1 * 30 = 30 tokens
Destination 3 receives: 1 * 20 = 20 tokens Total distributed: 100 tokens

Now, if there are 101 tokens to distribute:

tokensPerShare = 101 / 100 = 1 (due to integer division)
Destination 1 receives: 1 * 50 = 50 tokens
Destination 2 receives: 1 * 30 = 30 tokens
Destination 3 receives: 1 * 20 = 20 tokens Total distributed: 100 tokens, with 1 token remaining undistributed

Fix

To address this issue, implement a two-step distribution process:

Distribute using integer division as currently implemented.
Calculate the remaining undistributed tokens and distribute them to destinations in order of their share size until all tokens are distributed.

Here's a code snippet illustrating the fix:

uint256 distributedAmount = 0;
for (uint256 i = 0; i < numTransfers; ++i) {
    IERC20Upgradeable(address(erc20)).safeTransferFrom(
        caller,
        transfers[i].addrTo,
        transfers[i].amount
    );
    distributedAmount += transfers[i].amount;
}

// Distribute remaining tokens
uint256 remainingTokens = amount - distributedAmount;
for (uint256 i = 0; i < numTransfers && remainingTokens > 0; ++i) {
    uint256 additionalAmount = Math.min(1, remainingTokens);
    IERC20Upgradeable(address(erc20)).safeTransferFrom(
        caller,
        transfers[i].addrTo,
        additionalAmount
    );
    remainingTokens -= additionalAmount;
}

This fix ensures that all tokens are distributed, maintaining fairness and preventing any loss of funds due to rounding errors.

Assessed type

Math

code-423n4 / 2024-07-reserve-findings

Rounding Errors in distribute Function Cause Undistributed Tokens and Potential Fund Loss #97