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

0 stars 0 forks source link

Oracle error introduced in the `issuancePremium`, could result in erroneous prices for the auctions at a particular timestamp #225

Closed c4-bot-7 closed 1 month ago

c4-bot-7 commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/mixins/RecollateralizationLib.sol#L299-L300 https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/mixins/RecollateralizationLib.sol#L336-L338 https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/plugins/assets/RTokenAsset.sol#L66

Vulnerability details

Impact

In the BackingManager.rebalance function the RecollateralizationLib.prepareRecollateralizationTrade function is called. In the prepareRecollateralizationTrade function the RecollateralizationLib.nextTradePair function is called to determine the trading parameters for the new trade auction which is going to be initiated to rebalance the basket.

The RecollateralizationLib.nextTradePair function calls the Asset.price() function of each of the registered assets in the reserve protocol as shown below:

            if (ctx.bals[i].gt(needed)) {
                (uint192 low, uint192 high) = reg.assets[i].price(); 
                if (ctx.bals[i].lt(needed)) {
                    uint192 amtShort = needed.minus(ctx.bals[i]); // {buyTok}
                    (uint192 low, uint192 high) = reg.assets[i].price(); // {UoA/buyTok}

The issue here is if multiple RTokenCollaterals are registered in the reserve protocol then the nextTradePair function will call the price() function of each of those RTokenCollaterals. As a result the RTokenAsset.tryPrice function will be called in the execution flow which in turn will call the basketHandler.price(true) as shown below:

        (uint192 lowBUPrice, uint192 highBUPrice) = basketHandler.price(true);

Since true is passed onto the basketHandler.price the issuancePremium is applied to the price calculation if the collateral is experiencing a depeg in tok/ref. This could introduce oracle error (2%-3% either way) in the savedPegPrice to the issuancePremium thus this error could get transferred to the collateral high price value of the RTokenCollateral. This could make the TradePrice.buyHigh and TradePrice.sellHigh parameters erroneous. As a result the initiated trade auctions during the BackingManager.rebalance execution will have erroneous trade parameters. This could lead to erroneous initiations in the DutchTrade.init auctions thus resulting in erroneous prices for the auctions at a particular timestamp (calculated in the DutchTrade._price function). This could be disadvantageous and loss of funds to either the rsrStakers or the RToken holders.

Proof of Concept

            if (ctx.bals[i].gt(needed)) {
                (uint192 low, uint192 high) = reg.assets[i].price(); // {UoA/sellTok}

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

                if (ctx.bals[i].lt(needed)) {
                    uint192 amtShort = needed.minus(ctx.bals[i]); // {buyTok}
                    (uint192 low, uint192 high) = reg.assets[i].price(); // {UoA/buyTok}

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

        (uint192 lowBUPrice, uint192 highBUPrice) = basketHandler.price(true); // {UoA/BU}

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/plugins/assets/RTokenAsset.sol#L66

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence when the price() function of a RTokenCollateral is called from the BackingManger contract (BackingManager.rebalance), it is recommended to ensure that the issuancePremium is not applied to the price calculation of the RTokenCollateral. This will ensure the oracle error which could be introduced via the issuancePremium onto the trade parameters of the auctions initiated during the BackingManager.rebalance execution, will be eliminated.

Assessed type

Other