Core Contracts

Relevant Context

The RToken contract is an interest-bearing token implementation that uses a liquidity index to track interest accrual. The contract scales token balances using this index, similar to Aave's aToken implementation. The scaling is meant to happen once during transfers to ensure proper interest accounting. This token is also used as a deposit token in the StabilityPool, where users can deposit RTokens to receive deCRVUSD tokens.

Finding Description

The RToken contract contains a critical issue where token amounts are incorrectly scaled twice during transfer operations. This occurs because:

1. The _update() function already scales the amount by dividing it by the normalized income:

1. However, both transfer() and transferFrom() functions also scale the amount before calling super.transfer() which ultimately calls _update():

This means the amount is divided by the normalized income twice, resulting in users receiving far fewer tokens than intended.

Additionally, transferFrom() compounds this issue by using the stale _liquidityIndex instead of getting the current normalized income from the reserve pool:

Impact Explanation

High. This double scaling bug causes all transfers to move significantly fewer shares than intended. Since RToken uses shares to track balances internally, and these shares are meant to represent a claim on the underlying assets plus accrued interest, the double scaling results in users transferring far fewer shares than they should.

The impact is particularly severe in the StabilityPool's deposit() function, which relies on safeTransferFrom to receive RTokens from users:

When users attempt to deposit into the StabilityPool:

1. They will transfer fewer RToken shares than intended due to the double scaling

2. However, the StabilityPool will record the full amount in userDeposits

3. The user will receive the full amount of deCRVUSD tokens
   This creates an accounting mismatch where the StabilityPool thinks it has more RToken shares than it actually does, potentially leading to system-wide insolvency.

Likelihood Explanation

High. This issue affects all transfer operations and will occur every time users attempt to transfer tokens or deposit into the StabilityPool.

Proof of Concept

Let's demonstrate with a concrete example:

1. Assume normalized income is 2 RAY (2 * 10^27)

2. User A has 100 shares (representing 200 tokens at current exchange rate)

3. User A attempts to deposit 100 tokens to the StabilityPool

4. In transferFrom():

   • First scaling: 100 tokens → 50 shares (100 / 2)

5. In _update():

   • Second scaling: 50 shares → 25 shares (50 / 2)

6. Result:

   • StabilityPool receives only 25 shares (representing 50 tokens)

   • StabilityPool records deposit as 100 tokens in userDeposits

   • User receives deCRVUSD tokens calculated based on 100 tokens

7. This creates a 50 token deficit in the StabilityPool's actual vs. recorded balance

Recommendation

Remove the scaling operations from transfer() and transferFrom() since scaling is already handled in _update(). The functions should be simplified to:

This ensures amounts are scaled exactly once during the transfer process.