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

0 stars 0 forks source link

issue and redeem can be called in the same timestamp allowing an attacker to manipulate some state variables #34

Closed c4-bot-2 closed 1 month ago

c4-bot-2 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/RToken.sol#L94

Vulnerability details

To get Rtoken you basically have to stake some collateral this is do it through issue function:


 function issueTo(address recipient, uint256 amount) public notIssuancePausedOrFrozen {
        ...

        address issuer = _msgSender(); // OK to save: it can't be changed in reentrant runs

        ...

        // == Interactions: Create RToken + transfer tokens to BackingManager ==
        _scaleUp(recipient, amtBaskets, supply);

        for (uint256 i = 0; i < erc20s.length; ++i) {
            IERC20Upgradeable(erc20s[i]).safeTransferFrom(
                issuer,
                address(backingManager),
                deposits[i]
            );
        }
    }

[Link]

This successfully mint some token to the issuer, if then he don't want the token anymore, he just have to call some of the redeem functions.


function redeemTo(address recipient, uint256 amount) public notFrozen {
       ...

        address caller = _msgSender();

        ...
        uint192 baskets = _scaleDown(caller, amount);
        emit Redemption(caller, recipient, amount, baskets);

        (address[] memory erc20s, uint256[] memory amounts) = basketHandler.quote(
            baskets,
            false,
            FLOOR
        );

        // === Interactions ===

        for (uint256 i = 0; i < erc20s.length; ++i) {
            if (amounts[i] == 0) continue;

            // Send withdrawal
            // slither-disable-next-line arbitrary-send-erc20
            IERC20Upgradeable(erc20s[i]).safeTransferFrom(
                address(backingManager),
                recipient,
                amounts[i]
            );
        }
    }

[Link]

The functions that mint and burn tokens are _scaleUp and _scaleDown:


 function _scaleUp(
        address recipient,
        uint192 amtBaskets,
        uint256 totalSupply
    ) private {
        // take advantage of 18 decimals during casting
        uint256 amtRToken = totalSupply != 0
            ? amtBaskets.muluDivu(totalSupply, basketsNeeded) // {rTok} = {BU} * {qRTok} * {qRTok}
            : amtBaskets; // {rTok}
        emit BasketsNeededChanged(basketsNeeded, basketsNeeded + amtBaskets);
        basketsNeeded += amtBaskets; <----

        // Mint RToken to recipient
        _mint(recipient, amtRToken); <----
    }

[Link]

The problem is that those functions are modifying important variables that are used in the whole system as basketsNeeded, supply of the Rtoken issuanceThrottle and redemptionThrottle buffers. Allowing a user(attacker) to freely modify this variables can lead to several problems.

Note the attack depend on how much buying power an attacker could have in many cases we talking about millions.

Impact

Since an attacker could modified and play with this variables (basketsNeeded, supply of the Rtoken issuanceThrottle and redemptionThrottle buffers) he could DoS some function like:

Attacker need to has enough tokens to issue and then redeen in other tx, Attacker could Dos the function mention above front running some of this tx issuing and then backrunning redeeming making revert or return the function mention above.

Also attacker can front run and backrun other users trying to issue or redeem manipulating the supply and affecting the share conversion, Note that this is a problem specially because the issue and redeem function are not implementing any slippage protection.

There are other function that are relaying in the state variables mention above.

Proof of Concept

The description is self explanatory, basically an attacker can issue to modify this variables front running a target tx and then backrun the tx redeeming.

Since this Rtoken is in mainnet i decided to work with foundry fork. i have a little test for this if the judge need it. I decided not submit the test and the set up here because is a fork of the real project.

Tools Used

Manual, review

Recommended Mitigation Steps

Consider add some restriction to avoid users to issue and redeem in the same block timestamp, see the stRSR which has a draft to avoid user stake and unstake in the same timestamp

Assessed type

Other