M-02 MitigationConfirmed

[c4-bot-10]

Lines of code

Vulnerability details

See:

Finding	Mitigation
M-02: Broken assumptions can lead to the inability to seize RSR	Pull Request

Navigating to M-02 from the previous contest we can see that there is a vulnerability/wrong assumption in the seizeRSR function, i.e by manipulating staking and unstaking actions we can get to a situation where the contract is unable to seize RSR due to a division by zero error. This occurs due to broken assumptions about stakeRSR and totalStakes, allowing a scenario where totalStakes becomes zero while stakeRSR remains non-zero. The issue can be triggered through a specific sequence of actions involving unstaking, manipulating stake rates, and front-running seizeRSR calls, as shown in the updated POC here. Now the recommended mitigation includes enforcing an invariant that if totalStakes is zero, then stakeRSR must also be zero by not accepting 0 amount mints, alternatively Reserve could consider updating to update the stakeRate only if both stakeRSR and totalStakes are non-zero or update the stakeRate to FIX_ONE if either of these two is zero. This has been sufficiently mitigated in the pull request used to solve this, considering Reserve now updates stakeRate only if both stakeRSR and totalStakes are non-zero i.e:

    function seizeRSR(uint256 rsrAmount) external {
        _requireNotTradingPausedOrFrozen();
        _notZero(rsrAmount);

        address caller = _msgSender();
        require(caller == address(backingManager), "!bm");

        uint256 rsrBalance = rsr.balanceOf(address(this));
        require(rsrAmount <= rsrBalance, "seize exceeds balance");

        _payoutRewards();

        uint256 seizedRSR;
        uint192 initRate = exchangeRate();
        uint256 rewards = rsrRewards();

        // Remove RSR from stakeRSR
        uint256 stakeRSRToTake = (stakeRSR * rsrAmount + (rsrBalance - 1)) / rsrBalance;
        stakeRSR -= stakeRSRToTake;

        seizedRSR = stakeRSRToTake;

         // update stakeRate, possibly beginning a new stake era
-         if (stakeRSR != 0) {
+         if (stakeRSR != 0 && totalStakes != 0) {
             // Downcast is safe: totalStakes is 1e38 at most so expression maximum value is 1e56
             stakeRate = uint192((FIX_ONE_256 * totalStakes + (stakeRSR - 1)) / stakeRSR);
         }

        if (stakeRSR == 0 || stakeRate > MAX_STAKE_RATE) {
            seizedRSR += stakeRSR;
            beginEra();
        }

        // Remove RSR from draftRSR
        uint256 draftRSRToTake = (draftRSR * rsrAmount + (rsrBalance - 1)) / rsrBalance;
        draftRSR -= draftRSRToTake;
        seizedRSR += draftRSRToTake;

        // update draftRate, possibly beginning a new draft era
        if (draftRSR != 0) {
            // Downcast is safe: totalDrafts is 1e38 at most so expression maximum value is 1e56
            draftRate = uint192((FIX_ONE_256 * totalDrafts + (draftRSR - 1)) / draftRSR);
        }

        if (draftRSR == 0 || draftRate > MAX_DRAFT_RATE) {
            seizedRSR += draftRSR;
            beginDraftEra();
        }

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

        // Transfer RSR to caller
        emit ExchangeRateSet(initRate, exchangeRate());
        IERC20Upgradeable(address(rsr)).safeTransfer(caller, seizedRSR);
    }

[c4-judge]

thereksfour marked the issue as satisfactory

[c4-judge]

thereksfour marked the issue as confirmed for report