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

5 stars 4 forks source link

RTokens issuances and redemptions at full throttle can fail when Furnace holds a balance #25

Closed c4-bot-9 closed 3 months ago

c4-bot-9 commented 3 months ago

Lines of code

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

Vulnerability details

When users want to redeem or issue a large amount of tokens, they are likely to make use of the issuanceAvailable and redemptionAvailable functions:

File: RToken.sol
428:     /// @return {qRTok} The maximum issuance that can be performed in the current block
429:     function issuanceAvailable() external view returns (uint256) {
430:         return issuanceThrottle.currentlyAvailable(issuanceThrottle.hourlyLimit(totalSupply()));
431:     }
432: 
433:     /// @return available {qRTok} The maximum redemption that can be performed in the current block
434:     function redemptionAvailable() external view returns (uint256 available) {
435:         uint256 supply = totalSupply();
436:         available = redemptionThrottle.currentlyAvailable(redemptionThrottle.hourlyLimit(supply));
437:         if (supply < available) available = supply;
438:     }

Let's now imagine a user uses these values to issue/redeem the maximum RToken amount they can.

Whether they call issue, issueTo, redeem, redeemTo, or redeemCustom, the assetRegistry.refresh() function is inevitably called.

In turn, assetRegistry.refresh() calls basketHandler.trackStatus() which calls refresh() for all the assets registered in the Rtoken. Among these, there is RTokenAsset, whose refresh() function calls furnace.melt():

File: RTokenAsset.sol
87:     function refresh() public virtual override {
88:         // No need to save lastPrice; can piggyback off the backing collateral's saved prices
89: 
90:         furnace.melt();
91:         if (msg.sender != address(assetRegistry)) assetRegistry.refresh();
92: 
93:         cachedOracleData.cachedAtTime = 0; // force oracle refresh
94:     }

which in turn burns part of the RTokens held:

File: Furnace.sol
65:     function melt() public {
66:         if (uint48(block.timestamp) < uint64(lastPayout + 1)) return;
67: 
68:         // # of whole periods that have passed since lastPayout
69:         uint48 numPeriods = uint48((block.timestamp) - lastPayout);
70: 
71:         // Paying out the ratio r, N times, equals paying out the ratio (1 - (1-r)^N) 1 time.
72:         uint192 payoutRatio = FIX_ONE.minus(FIX_ONE.minus(ratio).powu(numPeriods));
73: 
74:         uint256 amount = payoutRatio.mulu_toUint(lastPayoutBal);
75: 
76:         lastPayout += numPeriods;
77:         lastPayoutBal = rToken.balanceOf(address(this)) - amount;
78:         if (amount != 0) rToken.melt(amount);
79:     }

Burning RTokens reduces its supply, and with it, the throttle limit, causing any calls at the limit or slightly below to fail.

Impact

RTokens issuances and redemptions at full throttle or slightly lower can fail when Furnace holds a balance, either from past distributions or from malicious external donations made to DoS the transactions.

Proof of Concept

Tools Used

Code review, Foundry

Recommended Mitigation Steps

Consider conservatively removing any RToken balance held by the Furnace from the supply considered for the throttle limit calculation in issuanceAvailable and redemptionAvailable.

Assessed type

DoS

akshatmittal commented 3 months ago
  1. The assumption that by view-ing a function you can get an accurate state of the chain is misguided, essentially you can imagine the same thing happening by another person issuing in the same block, or several other things in the protocol to do with the rate of RTokens.
  2. Additionally, melt does NOT impact the throttles and throttle is for external issuance and redemption.
c4-judge commented 3 months ago

thereksfour marked the issue as unsatisfactory: Invalid