`BackingManager` is unable to rebalance if RSR is not registered

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/mixins/RecollateralizationLib.sol#L358

Vulnerability details

Description

The BackingManager takes care of the backing of its RToken by constantly balancing the amounts of the respective underlying assets with trades. It tries to sell the token it has more of than necessary for the token it has less of than needed. If it does not have a good candidate token to sell, it will choose to sell the RSR token. However, if the RSR is not registered in the AssetRegistry, it cannot rebalance.

Proof of concept

If the respective RToken system does not intend to use the RSR token as revenue or a collateral asset, the owners will not register it to the AssetRegistry. However, if it is not registered in the AssetRegistry, then if such a scenario as the above one occurs, the BackingManager would be unable to rebalance due to an out-of-bounds revert here. Note that this behavior is documented for the further array accesses. Still, it should not behave this way, as it breaks core functionality.

Normally, if RSR was registered but the BackingManager did not have enough of it, it would choose to go for the haircut (i.e., decrease the number of baskets needed). There should not be a limitation like this if the system owners do not wish to slash governance or simply do not wish to include the RSR.

Impact and likelihood

Since this issue breaks a core protocol functionality, this issue should be judged as MEDIUM severity.

Recommendation

Consider checking whether the rsrIndex was assigned a correct value if it should be used as the sell asset:

        // Use RSR if needed
-       if (address(trade.sell) == address(0) && address(trade.buy) != address(0)) {
+       if (address(trade.sell) == address(0) && address(trade.buy) != address(0) && rsrIndex != reg.erc20s.length) {
            (uint192 low, uint192 high) = reg.assets[rsrIndex].price(); // {UoA/RSR}

            // if rsr does not have a registered asset the below array accesses will revert
            if (
                high != 0 &&
                TradeLib.isEnoughToSell(
                    reg.assets[rsrIndex],
                    ctx.bals[rsrIndex],
                    low,
                    ctx.minTradeVolume
                )
            ) {
                trade.sell = reg.assets[rsrIndex];
                trade.sellAmount = ctx.bals[rsrIndex];
                trade.prices.sellLow = low;
                trade.prices.sellHigh = high;
            }
        }

Assessed type

Error

[thereksfour]

rsr will make sure to be registered, unregistering rsr will be malicious governance behavior

        IAsset[] memory assets = new IAsset[](2);
        assets[0] = new RTokenAsset(components.rToken, params.rTokenMaxTradeVolume);
        assets[1] = rsrAsset;

        // Init Asset Registry
        main.assetRegistry().init(main, assets);

[c4-judge]

thereksfour marked the issue as unsatisfactory: Invalid