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

0 stars 0 forks source link

Potential Loss of Small Surplus Amounts in Revenue Distribution #251

Closed c4-bot-4 closed 1 month ago

c4-bot-4 commented 1 month ago

Lines of code

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

Vulnerability details

Vulnerability Details

The forwardRevenue function in the BackingManagerP1 contract is responsible for distributing surplus assets to RToken and RSR holders. It calculates the excess amount of each asset and attempts to distribute it proportionally.

The current implementation may fail to distribute small surplus amounts due to integer division rounding down to zero, potentially leading to asset accumulation in the contract over time.

In the distribution logic, tokensPerShare is calculated as:

uint256 delta = bal.minus(req).shiftl_toUint(int8(asset.erc20Decimals()));
uint256 tokensPerShare = delta / (totals.rTokenTotal + totals.rsrTotal);
if (tokensPerShare == 0) continue;

When delta (the surplus amount) is smaller than (totals.rTokenTotal + totals.rsrTotal), tokensPerShare becomes zero due to integer division. This results in skipping the distribution for that asset entirely.

Impact

  1. Small surplus amounts of assets may accumulate in the contract without being distributed.
  2. Over time, this could lead to a significant value being stuck, especially for high-value tokens or in high-volume scenarios.
  3. The system's efficiency in distributing all available surplus is reduced.

Scenario

Consider a case where there's a surplus of 99 units of a token, and (totals.rTokenTotal + totals.rsrTotal) is 100. The tokensPerShare would be 0, resulting in no distribution and 99 units remaining stuck in the contract.

Proposed Fix

Implement a minimum distribution mechanism to ensure that small surpluses are always distributed, even when tokensPerShare rounds down to zero.

uint256 delta = bal.minus(req).shiftl_toUint(int8(asset.erc20Decimals()));
uint256 tokensPerShare = delta / (totals.rTokenTotal + totals.rsrTotal);

if (tokensPerShare == 0 && delta > 0) {
    // Distribute minimum amounts to both traders if possible
    uint256 minDistribution = 1;
    if (delta >= 2 * minDistribution) {
        erc20s[i].safeTransfer(address(rsrTrader), minDistribution);
        erc20s[i].safeTransfer(address(rTokenTrader), minDistribution);
        delta -= 2 * minDistribution;
    }

    // Distribute remaining to the trader with higher total
    if (delta > 0) {
        address recipient = totals.rsrTotal >= totals.rTokenTotal ? address(rsrTrader) : address(rTokenTrader);
        erc20s[i].safeTransfer(recipient, delta);
    }
} else if (tokensPerShare > 0) {
    // Existing distribution logic...
}

This approach ensures that even small surpluses are distributed, maintaining system efficiency and preventing asset accumulation.

Assessed type

Context