The supply throttle checking in issuance should be performed after the `_scaleUp`.

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

The supply throttle check in issuance is performed before the _scaleUp, where _mint is called within _scaleUp. This is problematic:

1. The supply throttle check will fail if current total supply is 0. This occurs when it is the first issuance of the RToken, or when all RTokens have been redeemed before.
2. The current available amount in throttle check is less than it should. Since the throttle checking is used to limit the issuance per hour, the current issuance should be taken into account when checking the limit for this hour. However, the check only considers the total supply before this issuance.

Proof of Concept

The supply used for throttle checking is the current total supply of RToken (i.e. totalSupply(), L118). It does not contain the amount of current issuance, as the minting of RToken is performed in _scaleUp (L496), which is called after the throttle checking.

// Function: RToken.sol#issueTo()
105:    function issueTo(address recipient, uint256 amount) public notIssuancePausedOrFrozen {
            ...
116:        // Ensure basket is ready, SOUND and not in warmup period
117:        require(basketHandler.isReady(), "basket not ready");
118:@>      uint256 supply = totalSupply();
119:
120:        // Revert if issuance exceeds either supply throttle
121:@>      issuanceThrottle.useAvailable(supply, int256(amount)); // reverts on over-issuance
122:@>      redemptionThrottle.useAvailable(supply, -int256(amount)); // shouldn't revert
            ...
145:        // == Interactions: Create RToken + transfer tokens to BackingManager ==
146:@>      _scaleUp(recipient, amtBaskets, supply);
            ...
155:    }

// Function: RToken.sol#_scalUp()
483:    function _scaleUp(
484:        address recipient,
485:        uint192 amtBaskets,
486:        uint256 totalSupply
487:    ) private {
            ...
495:        // Mint RToken to recipient
496:        _mint(recipient, amtRToken);
497:    }

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

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

Total supply is then used to calculate the hourly limit of the supply throttle (L46 => L88), and the limit is then used to calculate the available amount to use (L49 => L75). If current total supply is 0, the available amount will be 0, and the supply throttle checking will fail (L58).

// Function: Throttle.sol#useAvailable()
37:    function useAvailable(
38:        Throttle storage throttle,
39:        uint256 supply,
40:        int256 amount
41:    ) internal {
42:        // untestable: amtRate will always be > 0 due to previous validations
43:        if (throttle.params.amtRate == 0 && throttle.params.pctRate == 0) return;
44:
45:        // Calculate hourly limit
46:@>      uint256 limit = hourlyLimit(throttle, supply); // {qRTok}
47:
48:        // Calculate available amount before supply change
49:@>      uint256 available = currentlyAvailable(throttle, limit);
50:
51:        // Update throttle.timestamp if available amount changed or at limit
52:        if (available != throttle.lastAvailable || available == limit) {
53:            throttle.lastTimestamp = uint48(block.timestamp);
54:        }
55:
56:        // Update throttle.lastAvailable
57:        if (amount > 0) {
58:@>          require(uint256(amount) <= available, "supply change throttled");
59:            available -= uint256(amount);
60:            // untestable: the final else statement, amount will never be 0
61:        } else if (amount < 0) {
62:            available += uint256(-amount);
63:        }
64:        throttle.lastAvailable = available;
65:    }

// Function: Throttle.sol#currentlyAvailable()
69:    function currentlyAvailable(Throttle storage throttle, uint256 limit)
70:        internal
71:        view
72:        returns (uint256 available)
73:    {
74:        uint48 delta = uint48(block.timestamp) - throttle.lastTimestamp; // {seconds}
75:@>      available = throttle.lastAvailable + (limit * delta) / ONE_HOUR;
76:        if (available > limit) available = limit;
77:    }

// Function: Throttle.sol#hourlyLimit()
80:    function hourlyLimit(Throttle storage throttle, uint256 supply)
81:        internal
82:        view
83:        returns (uint256 limit)
84:    {
85:        Params storage params = throttle.params;
86:
87:        // Calculate hourly limit as: max(params.amtRate, supply.mul(params.pctRate))
88:@>      limit = (supply * params.pctRate) / FIX_ONE_256; // {qRTok}
89:        if (params.amtRate > limit) limit = params.amtRate;
90:    }

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/libraries/Throttle.sol#L37-L65

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/libraries/Throttle.sol#L69-L77

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/libraries/Throttle.sol#L80-L90

Tools Used

VS Code

Recommended Mitigation Steps

In issuance, perform the supply throttle checking after the _scaleUp.

// Function: RToken.sol#issueTo()
function issueTo(address recipient, uint256 amount) public notIssuancePausedOrFrozen {
    ...
    // Ensure basket is ready, SOUND and not in warmup period
    require(basketHandler.isReady(), "basket not ready");
    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
    ...
    // == Interactions: Create RToken + transfer tokens to BackingManager ==
    _scaleUp(recipient, amtBaskets, supply);

+   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
    ...
}

Assessed type

Other

[tbrent]

It is correct for the throttle to accumulate before changing the supply, since it is a catch-up step. It should not happen after.

We can see straightforwardly that the following claim is incorrect because RTokens are successfully first minted all the time:

The supply throttle check will fail if current total supply is 0. This occurs when it is the first issuance of the RToken, or when all RTokens have been redeemed before.

[c4-judge]

thereksfour marked the issue as unsatisfactory: Invalid