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

5 stars 4 forks source link

`rsrRewardsAtLastPayout` is incorrectly updated to a smaller value in `seizeRSR`. #67

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

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

Vulnerability details

Impact

rsrRewardsAtLastPayout is incorrectly updated to a smaller value in seizeRSR. This results in the staker pool receiving less rewards in subsequent reward payouts than expected.

Proof of Concept

In function seizeRSR, the amount of seized stakeRSR, draftRSR, and rewards are calculated based on the seized ratio, where seizedRatio = ceil(rsrAmount / rsrBalance). At line 472, the seized rsrRewards is calculated as (rewards * rsrAmount + (rsrBalance - 1)) / rsrBalance, and is then added to the total seizedRSR. Then the seized rsrRewards should be subtracted from the current rsrRewards to update rsrRewardsAtLastPayout. However, it is the total seizedRSR is subtracted from the current rsrRewards instead of the seized rsrRewards. This results in rsrRewardsAtLastPayout being incorrectly updated to a smaller value.

// Function: StRSR.sol#seizeRSR()

471:        // Remove RSR from yet-unpaid rewards (implicitly)
472:        seizedRSR += (rewards * rsrAmount + (rsrBalance - 1)) / rsrBalance;
473:        rsrRewardsAtLastPayout = rsrRewards() - seizedRSR;

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

The rsrRewardsAtLastPayout is used in _payoutRewards to calculate the payout amount (L609-L610). If rsrRewardsAtLastPayout is incorrectly updated to a smaller value, the staker pool will receive less rewards than expected.

// Function: StRSR.sol#_payoutRewards()

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:        }

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

Tools Used

VS Code

Recommended Mitigation Steps

Update the rsrRewardsAtLastPayout by subtracting the seized rsrRewards from the current rsrRewards instead of the total seizedRSR.

// Function: StRSR.sol#seizeRSR()

        // Remove RSR from yet-unpaid rewards (implicitly)
-       seizedRSR += (rewards * rsrAmount + (rsrBalance - 1)) / rsrBalance;
-       rsrRewardsAtLastPayout = rsrRewards() - seizedRSR;
+       uint256 rewardsToTake = (rewards * rsrAmount + (rsrBalance - 1)) / rsrBalance;
+       seizedRSR += rewardsToTake;
+       rsrRewardsAtLastPayout = rsrRewards() - rewardsToTake

Assessed type

Other

tbrent commented 3 months ago

The rsrRewards() function already reflects the adjusted stakeRSR and draftRSR variables after seizure: https://github.com/reserve-protocol/protocol/blob/72fc1f6e41da01e733c0a7e96cdb8ebb45bf1065/contracts/p1/StRSR.sol#L694

c4-judge commented 2 months ago

thereksfour marked the issue as unsatisfactory: Invalid