If the current protocol is Under-Collateralized, users’ Funds may be Frozen.

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/RToken.sol#L183-L225 https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/RToken.sol#L253-L344

Vulnerability details

Impact

If the current protocol is under-collateralized, users’ funds may be Frozen due to missing of the bounding handling for each withdrawal in RToken.sol#redeemTo() function

Proof of Concept

The RToken.sol#redeemCustom() function performs the bounding handling for each withdrawal so that funds of users do not freeze.

function redeemCustom(
        ...
    ) external notFrozen {
        SNIP...

        // Bound each withdrawal by the prorata share, in case we're currently under-collateralized
297:    for (uint256 i = 0; i < erc20s.length; ++i) {
            // {qTok} = {qTok} * {qRTok} / {qRTok}
299:        uint256 prorata = mulDiv256(
300:            IERC20(erc20s[i]).balanceOf(address(backingManager)),
301:            amount,
302:            supply
303:        ); // FLOOR

305:        if (prorata < amounts[i]) amounts[i] = prorata;
306:    }

        // === Save initial recipient balances ===

        uint256[] memory pastBals = new uint256[](expectedERC20sOut.length);
        for (uint256 i = 0; i < expectedERC20sOut.length; ++i) {
            pastBals[i] = IERC20(expectedERC20sOut[i]).balanceOf(recipient);
            // we haven't verified this ERC20 is registered but this is always a staticcall
        }

        // === Interactions ===

        // Distribute tokens; revert if empty redemption
        {
            bool allZero = true;
            for (uint256 i = 0; i < erc20s.length; ++i) {
                if (amounts[i] == 0) continue; // unregistered ERC20s will have 0 amount
                if (allZero) allZero = false;

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

        // === Post-checks ===

        // Check post-balances
        for (uint256 i = 0; i < expectedERC20sOut.length; ++i) {
            uint256 bal = IERC20(expectedERC20sOut[i]).balanceOf(recipient);
            // we haven't verified this ERC20 is registered but this is always a staticcall
            require(bal - pastBals[i] >= minAmounts[i], "redemption below minimum");
        }
    }

However, the RToken.sol#redeemTo() function does not perform the bounding handling for each withdrawal.

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

        // === 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]
            );
        }
    }

Therefore, in case the protocol is currently under-collateralized, the transaction will be reverted. This may result in your funds being frozen, and in the worst case scenario, if the contract becomes inactive due to insufficient collateral, your collateral may become permanently unwithdrawalable.

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to add the boundling handling for each withdrawal in the redeemTo() function.

Assessed type

Other