Incorrect Exchange Rate Calculation in `_scaleUp()` Function Allows Inflated RToken Supply and Insufficient Collateral Backing

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RToken.sol#L483-L497 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RToken.sol#L89-L110

Vulnerability details

The issue() and issueTo() functions allow users to mint new RTokens by depositing the required collateral baskets. The contract tracks the number of collateral baskets needed (basketsNeeded) to fully collateralize the RToken supply and calculates the exchange rate as basketsNeeded / totalSupply.

There is an issue in the _scaleUp() function, which is called by issueTo() to mint new tokens and update the basketsNeeded value. The problem arises when minting tokens from a zero supply state. In this case, the function mints amtBaskets worth of RTokens instead of using the current exchange rate, allowing the effective exchange rate to decrease on subsequent mints.

Impact

• Minting more RTokens than intended for the deposited collateral, leading to an inflated supply and insufficient collateral backing.
• Incorrect accounting of collateral and RTokens in the system, potentially causing solvency issues for the protocol.

Proof of Concept

• The RToken contract is deployed with an initial zero supply and zero basketsNeeded.

• Alice calls issueTo(alice, 1), minting 1 RToken to her address. The _scaleUp() function is called with amtBaskets = 1, totalSupply = 0, and basketsNeeded = 0. Since totalSupply is zero, the function mints amtRToken = amtBaskets = 1 to Alice and updates basketsNeeded to 1.

• Bob calls issueTo(bob, 1), minting 1 RToken to his address. The _scaleUp() function is called with amtBaskets = 1, totalSupply = 1, and basketsNeeded = 1. The function calculates amtRToken as follows:

  uint256 amtRToken = amtBaskets.muluDivu(totalSupply, basketsNeeded);

  This results in amtRToken = 1 * 1 / 1 = 1, minting 1 RToken to Bob and updating basketsNeeded to 2.

The exchange rate has decreased from 1 basket per RToken (after Alice's mint) to 0.5 baskets per RToken (after Bob's mint).

Tools Used

Manual review

Recommended Mitigation Steps

The _scaleUp() function should establish a non-zero initial basketsNeeded value when minting from a zero supply state.

function _scaleUp(
    address recipient,
    uint192 amtBaskets,
    uint256 totalSupply
) private {
-   uint256 amtRToken = totalSupply != 0
-       ? amtBaskets.muluDivu(totalSupply, basketsNeeded) // {rTok} = {BU} * {qRTok} * {qRTok}
-       : amtBaskets; // {rTok}
+   uint256 amtRToken;
+   if (totalSupply == 0) {
+       amtRToken = amtBaskets;
+       basketsNeeded = amtBaskets;
+   } else {
+       amtRToken = amtBaskets.muluDivu(totalSupply, basketsNeeded);
+       basketsNeeded += amtBaskets;
+   }
    emit BasketsNeededChanged(basketsNeeded, basketsNeeded + amtBaskets);
-   basketsNeeded += amtBaskets;

    // Mint RToken to recipient
    _mint(recipient, amtRToken);
}

Assessed type

Math

[tbrent]

The PoC does not function as described.

The exchange rate has decreased from 1 basket per RToken (after Alice's mint) to 0.5 baskets per RToken (after Bob's mint).

The exchange rate has not decreased -- it is 1:1 between basketsNeeded and totalSupply.

[thereksfour]

This results in amtRToken = 1 * 1 / 1 = 1, minting 1 RToken to Bob and updating basketsNeeded to 2.

And _mint will update totalSupply to 2.

The exchange rate has decreased from 1 basket per RToken (after Alice's mint) to 0.5 baskets per RToken (after Bob's mint).

The exchange rate is still basketsNeeded / totalSupply = 2 / 2 = 1

[c4-judge]

thereksfour marked the issue as unsatisfactory: Invalid

[0xbrett8571]

I'll make sure to clarify and it might help. Even if it is not immediately evident in the simplistic arithmetic of basketsNeeded / totalSupply. The change in effective backing is the real issue and can lead to an inflated supply without appropriate collateral backing

1. Initial State and Exchange Rate Definition:
   The initial state has totalSupply = 0 and basketsNeeded = 0. The exchange rate is defined as basketsNeeded / totalSupply. The protocol starts in a state where no tokens are minted and no collateral is required.

2. Effect of the First Mint:

   • When Alice mints 1 RToken, _scaleUp() is called with amtBaskets = 1, totalSupply = 0, and basketsNeeded = 0.
   • Since totalSupply is zero, the function mints amtRToken = amtBaskets = 1 and updates basketsNeeded to 1.
   • After this, totalSupply is also 1 (because _mint() will increase the total supply by amtRToken), and basketsNeeded is 1.
   • The exchange rate is 1 basket per RToken, calculated as basketsNeeded / totalSupply = 1 / 1 = 1.

3. Effect of the Second Mint:

   • When Bob mints another 1 RToken, _scaleUp() is called with amtBaskets = 1, totalSupply = 1, and basketsNeeded = 1.
   • The function calculates amtRToken = amtBaskets.muluDivu(totalSupply, basketsNeeded), which simplifies to 1 * 1 / 1 = 1.
   • amtRToken = 1 is minted for Bob, and basketsNeeded is updated to 2.
   • After this mint, totalSupply becomes 2 (again, _mint() updates this), and basketsNeeded is 2.
   • The exchange rate is still 1 basket per RToken, calculated as basketsNeeded / totalSupply = 2 / 2 = 1.

I believe the confusion stems from a misunderstanding of how the exchange rate evolves when the protocol's state changes from zero to non-zero supply:

• Issue with Zero Initial State:
  When starting from a zero supply state, the first mint operation does not consider an exchange rate but instead sets an implicit one by directly using the amtBaskets value. This is problematic because subsequent mints use a different formula (amtBaskets.muluDivu(totalSupply, basketsNeeded)), which relies on the assumption that an initial exchange rate already exists.

• Decreasing Effective Exchange Rate:
  The effective exchange rate for new minters changes relative to the initial state. If you consider the sequence:

  • Alice mints 1 RToken for 1 basket (1:1).
  • Bob mints another 1 RToken but now contributes only 1 basket to a system that requires 2 baskets for 2 RTokens.

  The effective backing per RToken decreases after the first mint. Bob’s mint introduces a change in the system's collateralization ratio because it does not account for additional collateral proportional to the new total supply — the system ends up with a 1:1 ratio only when retrospectively recalculated, not in effective terms.

[thereksfour]

I believe this is a widespread way of setting initial exchange rates and don't see any vulnerabilities in it.