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

0 stars 0 forks source link

Attacker can exploit custom redemption to receive more value than intended (`RTokenP0::redeemCustom`) #72

Closed c4-bot-3 closed 1 month ago

c4-bot-3 commented 2 months ago

Lines of code

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

Vulnerability details

Description

The RTokenP1 contract implements a custom redemption mechanism through the redeemCustom() function, allowing users to redeem RTokens for a mix of previous baskets. This function is designed to provide flexibility in redemption, but it lacks crucial checks that could lead to potential exploitation.

The core issue lies in the assumption that previous baskets are not worth more than the current basket's target value. However, this assumption can be violated if a collateral token experiences an upward depeg, significantly increasing its value. In such a scenario, an attacker could exploit the redeemCustom() function to redeem RTokens for a combination of current and previous baskets, potentially receiving more value than intended.

The issue stems from the absence of checks in the redeemCustom() function to ensure that the collateral used is sound and that its price has not depegged upwards. Specifically, the function does not verify the current status or price of the collateral tokens being redeemed.

Key parts of the vulnerable code:

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

    (address[] memory erc20s, uint256[] memory amounts) = main
    .basketHandler()
    .quoteCustomRedemption(basketNonces, portions, basketsRedeemed);

    // ... (code omitted for brevity)

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

        if (amounts[i] > 0) {
            IERC20(erc20s[i]).safeTransferFrom(
                address(main.backingManager()),
                recipient,
                amounts[i]
            );
            allZero = false;
        }
    }

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

The function relies on the quoteCustomRedemption() function from the BasketHandler contract to determine the amounts of tokens to be redeemed. However, it does not perform any additional checks on the soundness or current value of these tokens before processing the redemption.

Impact

An attacker could exploit this mechanism to receive more value than intended, effectively diverting funds that should go to revenue traders or other protocol participants.

The impact is particularly severe in scenarios where a collateral token experiences a significant upward price movement. In such cases, the attacker could strategically redeem RTokens using a mix of current and previous baskets, potentially extracting more value than the RTokens are supposed to represent.

Proof of Concept

  1. The protocol includes a basket with Token X, initially pegged at 1 USD.
  2. Token X experiences an upward depeg, its value increasing to 2 USD.
  3. The protocol initiates a basket refresh, replacing Token X with Token Y in the current basket.
  4. Before the protocol can fully adjust (e.g., before forwardRevenue() is called), an attacker calls redeemCustom().
  5. The attacker specifies a redemption mix of 50% from the current basket (containing Token Y) and 50% from the previous basket (containing the appreciated Token X).
  6. For each RToken redeemed, the attacker receives 0.5 X + 0.5 Y, worth 1.5 USD, instead of the intended 1 USD.

Example call:

rToken.redeemCustom(
    attacker,
    1e18, // 1 RToken
    [currentNonce, previousNonce],
    [0.5e18, 0.5e18], // 50% each basket
    [tokenY, tokenX],
    [0, 0] // No minimum amounts
);

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this issue implement additional checks in the redeemCustom() function to ensure that the collateral used is sound and that its price has not depegged upwards. Here's a suggested modification:

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

    (address[] memory erc20s, uint256[] memory amounts) = main
    .basketHandler()
    .quoteCustomRedemption(basketNonces, portions, basketsRedeemed);

+   // Check collateral soundness and price
+   for (uint256 i = 0; i < erc20s.length; i++) {
+       require(main.basketHandler().isCollateralSound(erc20s[i]), "Unsound collateral");
+       require(main.basketHandler().getCollateralPrice(erc20s[i]) <= main.basketHandler().getPegPrice(erc20s[i]), "Collateral price too high");
+   }

    // ... (rest of the existing code)
}

These additional checks ensure that:

  1. All collateral tokens used in the redemption are currently considered sound by the protocol.
  2. The current price of each collateral token does not exceed its peg price, preventing exploitation of upward depegs.

Additionally, consider implementing a cooldown period after basket changes to allow the protocol time to adjust before allowing custom redemptions.

Assessed type

Other