Custom Redemption Vulnerability in Reserve Protocol Allows Value Extraction

[c4-bot-5]

Lines of code

https://github.com/reserve-protocol/protocol/blob/c4ec2473bbcb4831d62af55d275368e73e16b984/contracts/p1/RToken.sol#L245-L338
https://github.com/reserve-protocol/protocol/blob/c4ec2473bbcb4831d62af55d275368e73e16b984/contracts/p1/BasketHandler.sol#L437-L440

Vulnerability details

Impact

The custom redemption feature, implemented in the RTokenP1 contract, allows users to redeem RTokens in exchange for a mix of current and previous baskets. This feature assumes that previous baskets aren't worth more than the target value of the current basket. However, this assumption can be violated if a collateral asset in a previous basket has appreciated significantly.
As funds intended for revenue traders are reduced, it could impact the protocol's ability to maintain its peg and provide returns to RSR stakers.

Proof of Concept

https://github.com/reserve-protocol/protocol/blob/c4ec2473bbcb4831d62af55d275368e73e16b984/contracts/p1/RToken.sol#L245-L338
https://github.com/reserve-protocol/protocol/blob/c4ec2473bbcb4831d62af55d275368e73e16b984/contracts/p1/BasketHandler.sol#L437-L440

Consider the following scenario:

• The basket contains token X, which is supposed to be pegged to value c.
• Token X depegs upwards and is now worth 2c.
  After delayUntilDefault passes, the basket gets disabled.
• A basket refresh is executed, and token Y is introduced as the backup token.
  Half of token X is traded for the required Y tokens.
• Before anyone calls 'forwardRevenue', an attacker calls custom redemption with half from the current basket and half from the previous one.
• The attacker receives 0.5X + 0.5Y per each RToken, which is worth 1.5c, instead of the intended 1c value.
  This shown here, this code does not check if the collateral used is sound or if its price is higher than the peg price, allowing the exploit

// code from RToken.sol
function redeemCustom(
    address recipient,
    uint256 amount,
    uint48[] memory basketNonces,
    uint192[] memory portions,
    address[] memory expectedERC20sOut,
    uint256[] memory minAmounts
) external notFrozen {
    // ... (checks and state updates)

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

    // ... (token transfers)
}

// Relevant code from BasketHandler.sol
function quoteCustomRedemption(
    uint48[] memory basketNonces,
    uint192[] memory portions,
    uint192 amount
) external view returns (address[] memory erc20s, uint256[] memory quantities) {
    // ... (calculation logic)
}

Tools Used

manual code review

Recommended Mitigation Steps

Implement additional checks in the redeemCustom function to ensure that redemptions using previous baskets don't allow extracting more value than intended. You could add a mechanism to adjust redemption values based on current market prices, not just target prices, to prevent exploitation of appreciated assets. Implementation of a "staleness" check for previous baskets, potentially disallowing redemptions from baskets that are too old or have experienced significant price changes. Consider adding a price ceiling check in the quoteCustomRedemption function to cap the value that can be extracted from appreciated assets.

Assessed type

Other