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

0 stars 0 forks source link

Attacker can Inflate RToken Supply (`BackingManagerP1::forwardRevenue`) #44

Closed c4-bot-7 closed 1 month ago

c4-bot-7 commented 2 months ago

Lines of code

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

Vulnerability details

Description

The forwardRevenue function in the BackingManagerP1 contract is designed to forward revenue to RevenueTraders and mint new RTokens based on the current collateral holdings. The function calculates the number of baskets that can be minted based on the current holdings (basketsHeld.bottom) divided by (FIX_ONE + backingBuffer). If the calculated baskets value is greater than the current basketsNeeded, it mints the difference in RTokens.

This calculation does not adequately account for the actual increase in collateral value since the last revenue distribution. An attacker could manipulate the basketsHeld.bottom value by front-running the forwardRevenue transaction, temporarily inflating the balance of collateral tokens. This would result in a higher baskets value, leading to the minting of more RTokens than should be allowed. The attacker could then remove the extra collateral, leaving the system with more RTokens than it has backing for.

The relevant code is as follows:

File: BackingManager.sol
179:     function forwardRevenue(IERC20[] calldata erc20s) external nonReentrant {

    // ... (earlier code omitted for brevity)

219:         // Mint revenue RToken
220:         // Keep backingBuffer worth of collateral before recognizing revenue
221:         uint192 baskets = (basketsHeld.bottom.div(FIX_ONE + backingBuffer));
222:         if (baskets > rToken.basketsNeeded()) {
223:             rToken.mint(baskets - rToken.basketsNeeded());
224:         }

    // ... (remaining code omitted)
}

Impact

Unauthorized inflation of the RToken supply could lead to potential destabilization of the entire system. The imbalance between the RToken supply and the collateral backing could undermine the protocol's integrity and result in a loss of trust among users. This vulnerability can be exploited by an attacker through front-running.

Proof of Concept

  1. Attacker deposits collateral: An attacker deposits a large amount of collateral just before the forwardRevenue function is called.
  2. Inflated baskets value: This inflates the basketsHeld.bottom value, leading to a higher baskets value.
  3. Unauthorized minting: The function mints more RTokens than should be allowed based on the inflated baskets value.
  4. Collateral removal: The attacker then withdraws the extra collateral, leaving the system under-collateralized.

Tools Used

Manual review

Recommended Mitigation Steps

Implement a more robust mechanism for calculating the amount of RTokens to mint, ensuring it is proportional to the actual increase in collateral value. Here’s a suggested fix:

function forwardRevenue(IERC20[] calldata erc20s) external nonReentrant {
    // ... (earlier code omitted for brevity)

-    // Mint revenue RToken
-    // Keep backingBuffer worth of collateral before recognizing revenue
-    uint192 baskets = (basketsHeld.bottom.div(FIX_ONE + backingBuffer));
-    if (baskets > rToken.basketsNeeded()) {
-        rToken.mint(baskets - rToken.basketsNeeded());
-    }

+    // Calculate previous and current collateral values
+    uint192 previousCollateralValue = // ... (calculate previous total collateral value)
+    uint192 currentCollateralValue = // ... (calculate current total collateral value)
+    uint192 collateralIncrease = currentCollateralValue.minus(previousCollateralValue);
+
+    // Calculate the current RToken supply
+    uint192 currentRTokenSupply = rToken.totalSupply();
+    
+    // Calculate the amount of RTokens to mint based on the increase in collateral value
+    uint192 rTokensToMint = collateralIncrease.mul(currentRTokenSupply).div(previousCollateralValue);
+
+    // Mint RTokens if there is an increase in collateral value
+    if (rTokensToMint > 0) {
+        rToken.mint(rTokensToMint);
+    }

    // ... (remaining code omitted)
}

This approach ensures that the minting of RTokens is proportional to the actual increase in collateral value, thereby preventing unauthorized inflation of the RToken supply.

Assessed type

Other