Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
c4-bot-6 opened this issue Aug 19, 2024 · 0 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working 🤖_57_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-6
Copy link
Contributor

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

@c4-bot-6 c4-bot-6 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Aug 19, 2024
c4-bot-7 added a commit that referenced this issue Aug 19, 2024
@c4-bot-12 c4-bot-12 added the 🤖_57_group AI based duplicate group recommendation label Aug 19, 2024
howlbot-integration bot added a commit that referenced this issue Aug 20, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working 🤖_57_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants