Throttle rate is applied incorrectly.

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-reserve/tree/main/contracts/p1/RToken.sol#L356-L359 https://github.com/code-423n4/2024-07-reserve/tree/main/contracts/p1/RToken.sol#L370-L375 https://github.com/code-423n4/2024-07-reserve/tree/main/contracts/p1/RToken.sol#L387-L391

Vulnerability details

Impact

Throttle rate will be applied incorrectly. For instance, the RToken can be issued more than issuance throttle settings or can be redeemed less than redemption throttle settings.

Proof of Concept

RToken updates the available amount and last update time of throttle limit for issueance and redemption in issueTo, redeemTo and redeemCustom functions but not update them in mint, melt and dissolve functions. For instance, mint function doesn't issue tokens to users but increase the totalSupply of RToken. Since the calculation of available amount depends on totalSupply, this causes the incorrect applying of throttle rate.

Vulnerability Detail

RToken update and check the throttle in the issueTo function as follows.

    function issueTo(address recipient, uint256 amount) public notIssuancePausedOrFrozen {
        --- SKIP ---
        uint256 supply = totalSupply();

        // Revert if issuance exceeds either supply throttle
        issuanceThrottle.useAvailable(supply, int256(amount)); // reverts on over-issuance
        redemptionThrottle.useAvailable(supply, -int256(amount)); // shouldn't revert
        --- SKIP ---
    }

Then the totalSupply is used in Throttle.sol#useAvailable function as follows.

    function useAvailable(
        Throttle storage throttle,
        uint256 supply,
        int256 amount
    ) internal {
        // untestable: amtRate will always be > 0 due to previous validations
        if (throttle.params.amtRate == 0 && throttle.params.pctRate == 0) return;

        // Calculate hourly limit
46:     uint256 limit = hourlyLimit(throttle, supply); // {qRTok}

        // Calculate available amount before supply change
49:     uint256 available = currentlyAvailable(throttle, limit);

        // Update throttle.timestamp if available amount changed or at limit
        if (available != throttle.lastAvailable || available == limit) {
            throttle.lastTimestamp = uint48(block.timestamp);
        }

        // Update throttle.lastAvailable
        if (amount > 0) {
            require(uint256(amount) <= available, "supply change throttled");
            available -= uint256(amount);
            // untestable: the final else statement, amount will never be 0
        } else if (amount < 0) {
            available += uint256(-amount);
        }
        throttle.lastAvailable = available;
    }

totalSupply is used to calculate limit in L46 and then limit is used to calculate available in L49. currentlyAvailable function of L49 is the following.

    function currentlyAvailable(Throttle storage throttle, uint256 limit)
        internal
        view
        returns (uint256 available)
    {
74:     uint48 delta = uint48(block.timestamp) - throttle.lastTimestamp; // {seconds}
75:     available = throttle.lastAvailable + (limit * delta) / ONE_HOUR;
        if (available > limit) available = limit;
    }

As a result, we can see that the increasement of available amount depends on totalSupply and the duration of it (L74-L75). However, mint function increase the totalSupply but doesn't update the issuanceThrottle.lastTimestamp.

From this, the following scenario is available.

1. Assume that issuanceThrottle.params.amtRate is zero or small, so we can ignore it. And assume that issuanceThrottle.params.pctRate = 10%.
2. Assume that totalSupply of RToken is 1000 at t = 0. Then the hourly limit of issuance is 1000 * 10% = 100.
3. There is no issuance of RToken for an hour.
4. At t = 3599, 1000 of RToken is minted by backingManager. Then totalSupply is updated to 2000 now.
5. At t = 3600, A user can calls issueTo to issue RToken with amount = 200.
6. Since totalSupply = 1000 from t = 0 to t = 3599 and totalSupply = 2000 from t = 3599 to t = 3600, the hourly limit of issuance should be (1000 * 10% * 3599 + 2000 * 10% * 1) / 3600 = 100. However, the user issued 200 tokens in 1 hour which exceeds 100.

Tools Used

Manual Review

Recommended Mitigation Steps

Modify RToken.sol#mint function as follows.

    function mint(uint192 baskets) external {
        require(_msgSender() == address(backingManager), "not backing manager");
        _scaleUp(address(backingManager), baskets, totalSupply());
++      issuanceThrottle.useAvailable(totalSupply(), 0);
++      redemptionThrottle.useAvailable(totalSupply(), 0);
    }

Assessed type

Math

[tbrent]

~Confirming the issue, but think~ severity is not HIGH. No funds are at risk.

[akshatmittal]

Going back at this.

The throttle is supposed to apply to external interactions not internal ones, and the throttle works correctly in all of those cases.

You can obviously see people making arguments in both directions on when should and should not the throttle apply, which bring it down to the goals of the throttle in the first place.

[thereksfour]

Agree with sponsor, using throttle for internal operations is likely to result in incorrect workflow and introduce new issues. It is fine for throttle to be used to just restrict external operations.

[c4-judge]

thereksfour marked the issue as unsatisfactory: Invalid

[c4-judge]

thereksfour marked the issue as unsatisfactory: Invalid

[c4-judge]

thereksfour changed the severity to QA (Quality Assurance)

[c4-judge]

thereksfour marked the issue as grade-b