Integer Division Rounding in `forwardRevenue` Function Can Lead to Loss of Tokens (`BackingManager::forwardRevenue`)

[c4-bot-10]

Lines of code

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

Vulnerability details

Description

The [forwardRevenue function](https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/BackingManager.sol#L179-L266) in the BackingManager contract is responsible for distributing surplus assets to the rsrTrader and rTokenTrader. The function calculates the number of tokens to distribute per share using integer division, which can result in rounding down and potentially skipping the distribution of small surpluses. This issue arises because the calculation tokensPerShare = delta / (totals.rTokenTotal + totals.rsrTotal) uses integer division, which truncates any remainder. If the surplus (delta) is smaller than the sum of totals.rTokenTotal and totals.rsrTotal, tokensPerShare will be 0, and the function will skip the distribution for that token.

Detailed Description

The forwardRevenue function is designed to forward surplus assets to the rsrTrader and rTokenTrader. The function first calculates the number of tokens to distribute per share using the following code:

File: BackingManager.sol
247:                 uint256 tokensPerShare = delta / (totals.rTokenTotal + totals.rsrTotal);
248:                 if (tokensPerShare == 0) continue;

This calculation uses integer division, which truncates any remainder. If the surplus (delta) is smaller than the sum of totals.rTokenTotal and totals.rsrTotal, tokensPerShare will be 0, and the function will skip the distribution for that token. This can lead to an accumulation of undistributed assets in the BackingManager, which is not the intended behavior.

For example, consider the following scenarios:

1. Surplus of 100 tokens:

   • delta = 100
   • totals.rTokenTotal = 60
   • totals.rsrTotal = 40
   • tokensPerShare = 100 / (60 + 40) = 100 / 100 = 1
   • Distribution:
     • rsrTrader receives: 1 * 40 = 40 tokens
     • rTokenTrader receives: 1 * 60 = 60 tokens
   • Total distributed: 100 tokens

2. Surplus of 99 tokens:

   • delta = 99
   • totals.rTokenTotal = 60
   • totals.rsrTotal = 40
   • tokensPerShare = 99 / (60 + 40) = 99 / 100 = 0
   • Since tokensPerShare is 0, the function will continue to the next token without distributing anything.
   • Total distributed: 0 tokens

Impact

The integer division rounding issue in the forwardRevenue function can lead to a significant loss of tokens, especially when dealing with small amounts of surplus assets or large total shares. This can result in an accumulation of undistributed assets in the BackingManager, leading to a loss of value for token holders and stakers. Over time, this can lead to a significant accumulation of undistributed assets, which is not the intended behavior and can affect the fairness and efficiency of the distribution mechanism.

Proof of Concept

1. User has a surplus of 99 tokens.
2. totals.rTokenTotal is 60 and totals.rsrTotal is 40.
3. The function calculates tokensPerShare = 99 / (60 + 40) = 99 / 100 = 0.
4. Since tokensPerShare is 0, the function skips the distribution for that token.
5. The 99 tokens remain undistributed, leading to a loss of value.

Tools Used

Manual review

Recommended Mitigation Steps

To address this issue, the calculation should use a more precise approach. The following code provides a fix by calculating the exact share for each trader and ensuring that all surplus tokens are distributed proportionally:

- uint256 tokensPerShare = delta / (totals.rTokenTotal + totals.rsrTotal);
- if (tokensPerShare == 0) continue;
- 
- if (totals.rsrTotal != 0) {
-     erc20s[i].safeTransfer(address(rsrTrader), tokensPerShare * totals.rsrTotal);
- }
- if (totals.rTokenTotal != 0) {
-     erc20s[i].safeTransfer(address(rTokenTrader), tokensPerShare * totals.rTokenTotal);
- }

+ uint256 rsrShare = (delta * totals.rsrTotal) / (totals.rTokenTotal + totals.rsrTotal);
+ uint256 rTokenShare = delta - rsrShare;
+ 
+ if (rsrShare > 0) {
+     erc20s[i].safeTransfer(address(rsrTrader), rsrShare);
+ }
+ if (rTokenShare > 0) {
+     erc20s[i].safeTransfer(address(rTokenTrader), rTokenShare);
+ }

This approach ensures that all surplus tokens are distributed proportionally, even when dealing with small amounts. It eliminates the potential for large amounts of value to be left undistributed due to rounding issues.

Assessed type

Other