Incorrect rounding direction in `_scaleDown` Rtoken

[c4-bot-1]

Lines of code

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

Vulnerability details

we all now that solidity does not support float values does why division always round to the lower decimals. projects also have to round in favor of the project. In this case of reserve protocol there is a instance that can be exploited. Let see the _scaleDown function which is called by the redeems functions:

 function _scaleDown(address account, uint256 amtRToken) private returns (uint192 amtBaskets) {
        // D18{BU} = D18{BU} * {qRTok} / {qRTok}
        amtBaskets = basketsNeeded.muluDivu(amtRToken, totalSupply()); // FLOOR <-----
        emit BasketsNeededChanged(basketsNeeded, basketsNeeded - amtBaskets);
        basketsNeeded -= amtBaskets;

        // Burn RToken from account; reverts if not enough balance
        _burn(account, amtRToken);
    }

[Link]

As you can see the amtBaskets is rounding down this allow an attacker to burn a low amount of tokens but not decrement the basket need.

An attacker can do the next scenario:

1. issue rtokens increasing the basketsNeeded. 2, redeem a low amount of token burning his amount but not decreasing the basketsNeeded doing this in a loop until he burn all his rtokens.
2. Attacker successfully manipulated the basketsNeeded

Impact

basketsNeeded is a important variables in the system used in the basketHandler, backingManager and the same Rtoken manipulated this variables could lead to DoS, incorrect calculations and stealing funds. An example could be rebalance function in the backingManager which have the require below

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

Proof of Concept

See the _scaleDown:

 function _scaleDown(address account, uint256 amtRToken) private returns (uint192 amtBaskets) {
        // D18{BU} = D18{BU} * {qRTok} / {qRTok}
        amtBaskets = basketsNeeded.muluDivu(amtRToken, totalSupply()); // FLOOR <-----
        emit BasketsNeededChanged(basketsNeeded, basketsNeeded - amtBaskets);
        basketsNeeded -= amtBaskets;

        // Burn RToken from account; reverts if not enough balance
        _burn(account, amtRToken);
    }

[Link]

The amtBaskets is rounding to floor.

Tools Used

Manual

Recommended Mitigation Steps

Consider check if the amtBaskets rounded to 0 if that is the case then revert:

 function _scaleDown(address account, uint256 amtRToken) private returns (uint192 amtBaskets) {
        // D18{BU} = D18{BU} * {qRTok} / {qRTok}
        amtBaskets = basketsNeeded.muluDivu(amtRToken, totalSupply()); // FLOOR 
        if (amtBaskets===){revert;} <-----
        emit BasketsNeededChanged(basketsNeeded, basketsNeeded - amtBaskets);
        basketsNeeded -= amtBaskets;

        // Burn RToken from account; reverts if not enough balance
        _burn(account, amtRToken);
    }

Assessed type

Other

[jorgect207]

hey @thereksfour, thank so much for your judgin

I want to take a look of this issue, there is a unsafe rounding issue that is not been handling well that allow manipulate basketsNeeded which is an is a important variables in the system used in the basketHandler, backingManager and the Rtoken manipulated.

[thereksfour]

amtBaskets is used to calculate the collateral tokens sent to the user. In this case, the user burned more rtokens and got back fewer collateral tokens. This is by design. And basketsNeeded is not manipulated. It should be said that the attacker's "donation" makes rtoken worth more collateral tokens.