Lines of code

Vulnerability details

Impact

Function withdraw() in StRSR completes an account’s unstaking. but when calculated amount of RSR token is 0 code still burn user draftRSR and returns. This would cause users small amount of deposits to get burned and user won’t receive any funds. as withdraw() is callable by anyone a malicious attacker can call withdraw() for each item separately which result in 0 RSR amount and user would lose his funds. As users separate withdrawals can sum up and calculated rsrAmount would be bigger than 0 for user so contract shouldn’t burn the small amounts of user balance for nothing.

Proof of Concept

This is withdraw() code:

    function withdraw(address account, uint256 endId) external notPausedOrFrozen {
        // == Refresh ==
        assetRegistry.refresh();

        // == Checks + Effects ==
        require(basketHandler.fullyCollateralized(), "RToken uncollateralized");
        require(basketHandler.status() == CollateralStatus.SOUND, "basket defaulted");

        uint256 firstId = firstRemainingDraft[draftEra][account];
        CumulativeDraft[] storage queue = draftQueues[draftEra][account];
        if (endId == 0 || firstId >= endId) return;

        require(endId <= queue.length, "index out-of-bounds");
        require(queue[endId - 1].availableAt <= block.timestamp, "withdrawal unavailable");

        uint192 oldDrafts = firstId > 0 ? queue[firstId - 1].drafts : 0;
        uint192 draftAmount = queue[endId - 1].drafts - oldDrafts;

        // advance queue past withdrawal
        firstRemainingDraft[draftEra][account] = endId;

        // ==== Compute RSR amount
        uint256 newTotalDrafts = totalDrafts - draftAmount;
        // newDraftRSR: {qRSR} = {qDrafts} * D18 / D18{qDrafts/qRSR}
        uint256 newDraftRSR = (newTotalDrafts * FIX_ONE_256 + (draftRate - 1)) / draftRate;
        uint256 rsrAmount = draftRSR - newDraftRSR;

        if (rsrAmount == 0) return;

        // ==== Transfer RSR from the draft pool
        totalDrafts = newTotalDrafts;
        draftRSR = newDraftRSR;

        emit UnstakingCompleted(firstId, endId, draftEra, account, rsrAmount);

        // == Interaction ==
        IERC20Upgradeable(address(rsr)).safeTransfer(account, rsrAmount);
    }

As you can see function is callable by anyone and code would return when calculated rsrAmount is 0 and it would burn user draft items. This would cause users to lose funds if they withdraw small amounts even if those small amounts sums would create a bigger RSR Amount. code would burn those drafts separately and user would receive nothing. imagine this scenario:

1. user1 has has 20 draft token in 4 withdraw item and the ratio between draft to RSR is 5 to 1. (5 draft Token worth 1 RSR token).
2. attacker would call withdraw(user1, 1) and it code would calculated rsrAmount as 0 and burn 0 index withdraw item which includes 4 draft token and user would receive nothing.
3. then attacker would call withdraw(user1, 2), withdraw(user1, 3), withdraw(user1, 4) and withdraw(user1, 5) separately and again all user draft withdraw items would be burned and user would receive nothing.
4. if user called withdraw(user1, 5) he would have received 2 RSR token but attacker caused user to receive nothing. so code allows users to lose dusts and burns their draft tokens. this would cause wrong funds distribution and other users can receive more funds. even so if those dust sums up user can receive RSR amount.

the issue of wrong reward distribution happens passively and users lose their dusts and attacker can cause the issue intentionally.

Tools Used

VIM

Recommended Mitigation Steps

revert instead of the return when rsr amount is 0

The text was updated successfully, but these errors were encountered:

All reactions