Redeem pausing should not be checked when collecting redemptions

Summary

Checking if redeem is paused when collecting redemptions might incorrectly prevent users from getting their collateral back.

Vulnerability Detail

Collecting redemptions via the collectRedemption() allows users with queued redeems to get their collateral back after the redemption delay has passed:

// LibUbiquityPool.sol

function collectRedemption(
        uint256 collateralIndex
    ) internal returns (uint256 collateralAmount) {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        require(
            poolStorage.isRedeemPaused[collateralIndex] == false,
            "Redeeming is paused"
        );

                require(
            (
                poolStorage.lastRedeemedBlock[msg.sender].add(
                    poolStorage.redemptionDelayBlocks
                )
            ) <= block.number,
            "Too soon to collect redemption"
        );

        bool sendCollateral = false;

        if (
            poolStorage.redeemCollateralBalances[msg.sender][collateralIndex] >
            0
        ) {
            collateralAmount = poolStorage.redeemCollateralBalances[msg.sender][
                collateralIndex
            ];
            poolStorage.redeemCollateralBalances[msg.sender][
                collateralIndex
            ] = 0;
            poolStorage.unclaimedPoolCollateral[collateralIndex] = poolStorage
                .unclaimedPoolCollateral[collateralIndex]
                .sub(collateralAmount);
            sendCollateral = true;
        }

        // send out the tokens
        if (sendCollateral) {
            IERC20(poolStorage.collateralAddresses[collateralIndex])
                .safeTransfer(msg.sender, collateralAmount);
        }

}

As shown in the code snippet, collectRedemption() will ensure that redemptions are paused by checking the poolStorage.isRedeemPaused[collateralIndex] flag. It is a way of controlling that users don’t queue redemptions in unforseen events (and that’s why poolStorage.isRedeemPaused[collateralIndex] is correctly verified in the redeemDollar() function).

However, it is not correct to check the isRedeemPaused flag when collecting the redemptions, given that this process does not affect the protocol’s stability mechanisms in any way, and only serves as a way for users to actually get their collateral back after waiting for the redemption time to pass. In other words, there is no reason to justify users not being able to withdraw their queued collateral after being able to actually redeem by triggering redeemDollar() (which is the actual function that will burn uAD tokens and makes the stability mechanisms execute and properly function).

Impact

I consider this vulnerability to be of medium impact, given that the current protocol design might lead to users being unfairly prevented from withdrawing their collateral.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L482

Tool used

Manual Review

Recommendation

Remove the isRedeemPaused check from the collectRedemption() function.

Duplicate of #103

sherlock-audit / 2023-12-ubiquity-judging

0xadrii - Redeem pausing should not be checked when collecting redemptions #135