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

0 stars 0 forks source link

Users can receive incorrect redemption amounts leading to under-collateralization (`RTokenP1::redeemCustom`) #58

Closed c4-bot-9 closed 1 month ago

c4-bot-9 commented 2 months ago

Lines of code

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

Vulnerability details

Description

The redeemCustom() function in the RTokenP1 contract allows users to redeem their RTokens for a custom combination of historical baskets. This function is designed to provide flexibility in the redemption process, allowing users to specify the portions of different baskets they wish to redeem. However, the current implementation lacks crucial checks to ensure the correct distribution of redemption amounts and maintain proper collateralization.

The function performs several key steps:

  1. Validates input parameters and checks user balance
  2. Updates issuance and redemption throttles
  3. Calculates the number of baskets to be redeemed
  4. Obtains redemption amounts for specified baskets
  5. Prorates redemption amounts based on current balances
  6. Distributes tokens to the recipient

The root cause of the issue lies in the proration and distribution steps. The function does not verify that the total value of the redeemed assets matches the expected redemption value, nor does it check if the contract remains fully collateralized after the redemption. This oversight can lead to scenarios where users receive incorrect redemption amounts, and the contract becomes under-collateralized.

For example, in the proration step:

for (uint256 i = 0; i < erc20s.length; ++i) {
    uint256 prorata = mulDiv256(
        IERC20(erc20s[i]).balanceOf(address(backingManager)),
        amount,
        supply
    ); // FLOOR

    if (prorata < amounts[i]) amounts[i] = prorata;
}

This code adjusts the redemption amounts based on available balances but doesn't ensure that the total value of the adjusted amounts equals the expected redemption value.

Impact

The primary impact of this issue is the potential for under-collateralization and incorrect redemption amounts. If the calculated redemption amounts are not correctly distributed, the contract may end up under-collateralized, meaning that the backing assets may not fully cover the issued RTokens.

Proof of Concept

  1. User Action: A user calls the redeemCustom function with a specified amount and portions of different baskets.
  2. Redemption Calculation: The function calculates the redemption amounts using basketHandler.quoteCustomRedemption.
  3. Proration: The function prorates the redemption amounts based on the current balances of the backing manager.
  4. Incorrect Distribution: Due to inadequate checks, the calculated redemption amounts are not correctly distributed, leading to under-collateralization and incorrect redemption amounts.

Tools Used

Manual review

Recommended Mitigation Steps

To address these issues, we recommend implementing additional checks in the redeemCustom() function:

  1. Verify the total value of redeemed assets matches the expected redemption value.
  2. Ensure the contract remains fully collateralized after the redemption.

Here's a diff of the proposed changes:

function redeemCustom(
    address recipient,
    uint256 amount,
    uint48[] memory basketNonces,
    uint192[] memory portions,
    address[] memory expectedERC20sOut,
    uint256[] memory minAmounts
) external notFrozen {
    // ... [existing code] ...

    // ==== Prorate redemption ====
    for (uint256 i = 0; i < erc20s.length; ++i) {
        uint256 prorata = mulDiv256(
            IERC20(erc20s[i]).balanceOf(address(backingManager)),
            amount,
            supply
        ); // FLOOR

        if (prorata < amounts[i]) amounts[i] = prorata;
    }

+   // === Verify total redemption amount ===
+   uint256 totalRedemptionValue = 0;
+   for (uint256 i = 0; i < erc20s.length; ++i) {
+       totalRedemptionValue += amounts[i] * assetRegistry.getAsset(erc20s[i]).price();
+   }
+   require(totalRedemptionValue >= amount, "Redemption value too low");

    // ... [existing code for token distribution] ...

+   // === Verify contract remains collateralized ===
+   require(basketHandler.fullyCollateralized(), "Contract under-collateralized");
}

These additions ensure that the redemption process maintains the correct value distribution and preserves the contract's collateralization, significantly reducing the risk of financial discrepancies and system instability.

Assessed type

Other