Stale Basket Data Used in RTokenP1.redeemCustom() Can Lead to Incorrect Redemption Amounts

[c4-bot-1]

Lines of code

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

Vulnerability details

Vulnerability details:

The redeemCustom() function in the RTokenP1 contract allows users to redeem RTokens for a customized basket of backing collateral based on historical basket compositions. However, the function does not refresh the asset registry before calculating the redemption amounts using the historical basket data.

This can potentially lead to incorrect token distributions if the underlying assets' states (such as their price, decimals, etc.) have changed since the historical baskets were recorded. Using stale data could result in incorrect redemption amounts being calculated and transferred to the user.

Impact

Some users may receive more or less than their fair share of collateral when redeeming using historical baskets. The impact depends on the magnitude of state changes in the collateral tokens since the historical baskets were recorded. This could lead to an unfair distribution of collateral.

Proof of Concept

1. Set up Reserve Protocol contracts locally
2. Deploy RToken with a collateral basket
3. Simulate state changes in collateral tokens (e.g. price or decimals)
4. Have user call redeemCustom() with historical basket nonces/portions
5. Verify redemption amounts are based on stale data, resulting in incorrect amounts transferred compared to expected fair share

Tools Used

• Hardhat for local testing
• Foundry for POC

Recommended Mitigation Steps

Add an assetRegistry.refresh() call at the beginning of the redeemCustom() function. This ensures the latest state of the collateral tokens is fetched before calculating redemption amounts, mitigating the risk of using stale data.

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

    // ... existing logic ...
}

Assessed type

Invalid Validation