If the payout criteria are not met, payoutLastPaid should not be updated.

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/StRSR.sol#L594-L614

Vulnerability details

Impact

In function _payoutRewards, if the payout criteria are not met, payoutLastPaid should not be updated. However, the payoutLastPaid is actually updated even if the payout criteria are not met, which leads subsequent payouts to be calculated incorrectly.

Proof of Concept

In function _payoutRewards, the payout is done only if enough RSR (i.e. >= FIX_ONE) is staked. If not, the payout will not happen. The payoutLastPaid records the last time when rewards are paid out. Thus if the payout does not happen, payoutLastPaid should not be updated.

However, in the function _payoutRewards, the payoutLastPaid is always updated regardless of whether a payout happens or not. This is problematic if the totalStakes of the previous stakes are less than FIX_ONE. When the totalStakes after a new stake is greater than FIX_ONE, a payout will happen. At this time, the payoutLastPaid will be smaller than it should be. This leads to the payoutRatio and payout being smaller than they should be.

The payoutRatio is calculated as (1 - (1-rewardRatio)^numPeriods). As payoutLastPaid becomes larger, numPeriods becomes smaller, (1-rewardRatio)^numPeriods becomes larger, and therefore (1 - (1-rewardRatio)^numPeriods) becomes smaller.

// Function: StRSR.sol#_payoutRewards()

594:        if (block.timestamp < payoutLastPaid + 1) return;
595:@>      uint48 numPeriods = uint48(block.timestamp) - payoutLastPaid;
596:
597:        uint192 initRate = exchangeRate();
598:        uint256 payout;
599:
600:        // Do an actual payout if and only if enough RSR is staked!
601:        if (totalStakes >= FIX_ONE) {
602:            // Paying out the ratio r, N times, equals paying out the ratio (1 - (1-r)^N) 1 time.
603:            // Apply payout to RSR backing
604:            // payoutRatio: D18 = FIX_ONE: D18 - FixLib.powu(): D18
605:            // Both uses of uint192(-) are fine, as it's equivalent to FixLib.sub().
606:@>          uint192 payoutRatio = FIX_ONE - FixLib.powu(FIX_ONE - rewardRatio, numPeriods);
607:
608:            // payout: {qRSR} = D18{1} * {qRSR} / D18
609:            payout = (payoutRatio * rsrRewardsAtLastPayout) / FIX_ONE;
610:            stakeRSR += payout;
611:        }
612:
613:@>      payoutLastPaid += numPeriods;
614:        rsrRewardsAtLastPayout = rsrRewards();

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/StRSR.sol#L594-L614

Tools Used

VS Code

Recommended Mitigation Steps

Update payoutLastPaid only if totalStakes >= FIX_ONE.

// Function: StRSR.sol#_payoutRewards()

        if (block.timestamp < payoutLastPaid + 1) return;
        uint48 numPeriods = uint48(block.timestamp) - payoutLastPaid;

        uint192 initRate = exchangeRate();
        uint256 payout;

        // Do an actual payout if and only if enough RSR is staked!
        if (totalStakes >= FIX_ONE) {
            // Paying out the ratio r, N times, equals paying out the ratio (1 - (1-r)^N) 1 time.
            // Apply payout to RSR backing
            // payoutRatio: D18 = FIX_ONE: D18 - FixLib.powu(): D18
            // Both uses of uint192(-) are fine, as it's equivalent to FixLib.sub().
            uint192 payoutRatio = FIX_ONE - FixLib.powu(FIX_ONE - rewardRatio, numPeriods);

            // payout: {qRSR} = D18{1} * {qRSR} / D18
            payout = (payoutRatio * rsrRewardsAtLastPayout) / FIX_ONE;
            stakeRSR += payout;
+          payoutLastPaid += numPeriods;
        }

-       payoutLastPaid += numPeriods;
        rsrRewardsAtLastPayout = rsrRewards();

Assessed type

Math