The discrepency in issuancePremium accounting, during the issuance of RTokens and the available basket units calculation in the BackingManager, could lead to an incorrect assessment of the collateralization status

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/RToken.sol#L139-L143
https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/BasketHandler.sol#L499-L504
https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/BackingManager.sol#L124
https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/BasketHandler.sol#L620-L626
https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/BackingManager.sol#L132

Vulnerability details

Impact

There is a discrepancy between the calculation of the required collateral tokens during issuance of RToken (which includes the issuancePremium) and the calculation of the available basket units held by the BackingManager (which does not account for the issuancePremium).

During a depeg scenario, when the RToken.issueTo function is executed, the BasketHandler.quote function is called with applyIssuancePremium set to true. This ensures that the required collateral token quantities are adjusted to account for the issuance premium, effectively requiring more collateral tokens to be transferred to the BackingManager contract. The following code snippet from BasketHandler.quote depicts this scenario:

            if (applyIssuancePremium) {
                uint192 premium = issuancePremium(coll); // {1} always CEIL by definition

                // {tok} = {tok} * {1}
                if (premium > FIX_ONE) q = q.safeMul(premium, rounding);
            }

However, when the BackingManager.rebalance function is executed, the BasketHandler.basketsHeldBy function is called to determine the number of basket units held by the BackingManager. This function directly uses the calculated token quantity per basket unit and does not apply the issuancePremium to calculate the available basket units as shown below:

            uint192 q = _quantity(basket.erc20s[i], coll, CEIL); //@audit-info - token quantity per basket unit is rounded up
            if (q == FIX_MAX) return BasketRange(FIX_ZERO, FIX_MAX); //@audit-info - (bottom, top)

            // {BU} = {tok} / {tok/BU}
            uint192 inBUs = coll.bal(account).div(q); //@audit-info - basket unit amount is rounded down
            baskets.bottom = fixMin(baskets.bottom, inBUs);
            baskets.top = fixMax(baskets.top, inBUs);

This discrepancy can lead to a situation where the basketsHeld.bottom value (the number of whole basket units held by the BackingManager) is overestimated because it does not account for the additional collateral tokens required due to the issuance premium.

Consequently, the following condition in the BackingManager.rebalance function may incorrectly evaluate to true, even though the BackingManager does not hold enough collateral tokens to fulfill the basketsNeeded value after considering the issuance premium:

    if (basketsHeld.bottom >= rToken.basketsNeeded()) return;

This potential bug could lead to an incorrect assessment of the collateralization status as true and potentially allow the protocol to continue operating in an undercollateralized state assuming it is collateralized, which could put both RToken holders and RSR stakers at risk. It's important to note that this bug may have implications for the overall collateralization management and the safety of the Reserve Protocol.

Proof of Concept

        (address[] memory erc20s, uint256[] memory deposits) = basketHandler.quote(
            amtBaskets,
            true,
            CEIL
        );

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/RToken.sol#L139-L143

            if (applyIssuancePremium) {
                uint192 premium = issuancePremium(coll); // {1} always CEIL by definition

                // {tok} = {tok} * {1}
                if (premium > FIX_ONE) q = q.safeMul(premium, rounding);
            }

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/BasketHandler.sol#L499-L504

        BasketRange memory basketsHeld = basketHandler.basketsHeldBy(address(this));

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/BackingManager.sol#L124

            uint192 q = _quantity(basket.erc20s[i], coll, CEIL);
            if (q == FIX_MAX) return BasketRange(FIX_ZERO, FIX_MAX);

            // {BU} = {tok} / {tok/BU}
            uint192 inBUs = coll.bal(account).div(q);
            baskets.bottom = fixMin(baskets.bottom, inBUs);
            baskets.top = fixMax(baskets.top, inBUs);

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/BasketHandler.sol#L620-L626

        if (basketsHeld.bottom >= rToken.basketsNeeded()) return; // return if now capitalized

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/BackingManager.sol#L132

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

To mitigate this issue, it is recommended to modify the BasketHandler.basketsHeldBy function to incorporate the issuancePremium calculation when determining the available basket units. This would ensure that the basketsHeld.bottom value accurately reflects the number of basket units that can be fulfilled with the available collateral tokens, taking into account the issuance premium during depeg scenarios.

Assessed type

Other