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

0 stars 0 forks source link

Incorrect Validation of `portions` Sum May Lead to Incorrect Redemptions (`RTokenP1::redeemCustom`) #48

Closed c4-bot-2 closed 1 month ago

c4-bot-2 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 is responsible for redeeming tokens based on custom basket portions. This function checks that the sum of the portions array equals FIX_ONE to ensure the portions add up to 1. However, the current implementation sums the portions using a uint256 sum, which may not correctly handle the precision of FIX_ONE (a fixed-point number). This can lead to incorrect validation of the portions sum, potentially allowing for incorrect redemptions.

The redeemCustom function first refreshes the asset registry and checks that the amount to be redeemed is non-zero and does not exceed the caller's balance. It then sums the portions array using a uint256 sum and checks if the sum equals FIX_ONE. If the sum is incorrect due to precision issues, the function may incorrectly validate the portions, leading to potential financial discrepancies or exploits.

Relevant code:

function redeemCustom(
    address recipient,
    uint256 amount,
    uint48[] memory basketNonces,
    uint192[] memory portions,
    address[] memory expectedERC20sOut,
    uint256[] memory minAmounts
) external notFrozen {
    // == Refresh ==
    assetRegistry.refresh();

    // == Checks and Effects ==

    require(amount != 0, "Cannot redeem zero");
    require(amount <= balanceOf(_msgSender()), "insufficient balance");
    uint256 portionsSum;
    for (uint256 i = 0; i < portions.length; ++i) {
        portionsSum += portions[i];
    }
    require(portionsSum == FIX_ONE, "portions do not add up to FIX_ONE");

    // ... rest of the function
}

Impact

The incorrect validation of the portions sum can lead to incorrect redemptions, affecting the integrity of the token supply and the fairness of the redemption process.

Proof of Concept

  1. A user calls the redeemCustom function with an amount to redeem and a portions array that sums to FIX_ONE in fixed-point arithmetic but not in uint256 arithmetic.
  2. The function sums the portions using a uint256 sum and incorrectly validates the sum as equal to FIX_ONE.
  3. The function proceeds with the redemption process, potentially leading to incorrect redemptions and financial discrepancies.

Tools Used

Manual review

Recommended Mitigation Steps

To address this issue, the sum of portions should be checked using fixed-point arithmetic to ensure precision is maintained. Here is the recommended fix:

function redeemCustom(
    address recipient,
    uint256 amount,
    uint48[] memory basketNonces,
    uint192[] memory portions,
    address[] memory expectedERC20sOut,
    uint256[] memory minAmounts
) external notFrozen {
    // == Refresh ==
    assetRegistry.refresh();

    // == Checks and Effects ==

    require(amount != 0, "Cannot redeem zero");
    require(amount <= balanceOf(_msgSender()), "insufficient balance");

-   uint256 portionsSum;
+   uint192 portionsSum = 0;
    for (uint256 i = 0; i < portions.length; ++i) {
-       portionsSum += portions[i];
+       portionsSum = portionsSum.plus(portions[i]);
    }
    require(portionsSum == FIX_ONE, "portions do not add up to FIX_ONE");

    // ... rest of the function
}

Assessed type

Other