0xpiken - User might not able to collect collateral after successful redemption

[sherlock-admin]

0xpiken

medium

User might not able to collect collateral after successful redemption

Summary

User might not able to call collectRedemption() successfully even redeemDollar() is invoked successfully.

Vulnerability Detail

A eligible user has right to call redeemDollar() to redeem their collateral by burning their uAD tokens.
Once the collateral balance in the pool is enough, the amount of redeemed collateral will be locked immediately. user can collect it by calling collectRedemption() several blocks later (redemptionDelayBlocks).

Ubiquity allows AMO minters to borrow collateral to make yield in external protocols. The amount of collateral they can borrow is defined in freeCollateralBalance():

Returns free collateral balance (i.e. that can be borrowed by AMO minters)

The implementation of freeCollateralBalance() is below:

    function freeCollateralBalance(
        uint256 collateralIndex
    ) internal view returns (uint256) {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();
        return
            IERC20(poolStorage.collateralAddresses[collateralIndex])
                .balanceOf(address(this))
                .sub(poolStorage.unclaimedPoolCollateral[collateralIndex]);
    }

From above codes we can see, any collateral that has been redeemed but not collected by user should not be used for borrowing.
However, it was not accounted when amoMinterBorrow() is called:

    function amoMinterBorrow(uint256 collateralAmount) internal onlyAmoMinter {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        // checks the collateral index of the minter as an additional safety check
        uint256 minterCollateralIndex = IDollarAmoMinter(msg.sender)
            .collateralIndex();

        // checks to see if borrowing is paused
        require(
            poolStorage.isBorrowPaused[minterCollateralIndex] == false,
            "Borrowing is paused"
        );

        // ensure collateral is enabled
        require(
            poolStorage.isCollateralEnabled[
                poolStorage.collateralAddresses[minterCollateralIndex]
            ],
            "Collateral disabled"
        );

        // transfer
        IERC20(poolStorage.collateralAddresses[minterCollateralIndex])
            .safeTransfer(msg.sender, collateralAmount);
    }

Since unclaimedPoolCollateral is not accounted, amo minter can borrow up to the entire collateral balance in the pool.

Copy below codes into UbiquityPoolFacetTest.t.sol and run forge test --match-test testCollectRedemption_CollectRedemptionFail():

    function testCollectRedemption_CollectRedemptionFail() public {
        vm.prank(admin);
        ubiquityPoolFacet.setPriceThresholds(
            1000000, // mint threshold
            1000000 // redeem threshold
        );

        // user sends 100 collateral tokens and gets 99 Dollars (-1% mint fee)
        vm.prank(user);
        ubiquityPoolFacet.mintDollar(
            0, // collateral index
            100e18, // Dollar amount
            99e18, // min amount of Dollars to mint
            100e18 // max collateral to send
        );

        //@audit-info user redeems 99 Dollars for collateral successfully
        vm.prank(user);
        ubiquityPoolFacet.redeemDollar(
            0, // collateral index
            99e18, // Dollar amount
            90e18 // min collateral out
        );

        // wait 3 blocks for collecting redemption to become active
        vm.roll(3);

        //@audit-info ubiquityPoolFacet has 100e18 collateralToken
        assertEq(collateralToken.balanceOf(address(ubiquityPoolFacet)), 100e18);
        //@audit-info collateralToken can be borrowed is less than 100e18
        assertTrue(ubiquityPoolFacet.freeCollateralBalance(dollarAmoMinter.collateralIndex()) < 100e18);
        //@audit-info however, dollarAmoMinter can borrow up to 100e18 collateralToken
        vm.prank(address(dollarAmoMinter));
        ubiquityPoolFacet.amoMinterBorrow(100e18);
        //@audit-info collateralToken balance in ubiquityPoolFacet is 0
        assertEq(collateralToken.balanceOf(address(ubiquityPoolFacet)), 0);

        //@audit-info user has no way to collect redeemed collateralToken
        vm.prank(user);
        vm.expectRevert();
        ubiquityPoolFacet.collectRedemption(0);
    }

Impact

User might not able to call collectRedemption() successfully even redeemDollar() is invoked successfully.

Code Snippet

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

Tool used

Manual Review

Recommendation

Check if there is enough free balance for borrowing in collectRedemption():

    function amoMinterBorrow(uint256 collateralAmount) internal onlyAmoMinter {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        // checks the collateral index of the minter as an additional safety check
        uint256 minterCollateralIndex = IDollarAmoMinter(msg.sender)
            .collateralIndex();

        // checks to see if borrowing is paused
        require(
            poolStorage.isBorrowPaused[minterCollateralIndex] == false,
            "Borrowing is paused"
        );

        // ensure collateral is enabled
        require(
            poolStorage.isCollateralEnabled[
                poolStorage.collateralAddresses[minterCollateralIndex]
            ],
            "Collateral disabled"
        );
+       require(collateralAmount <= freeCollateralBalance(minterCollateralIndex), "No free collateral for borrowing");
        // transfer
        IERC20(poolStorage.collateralAddresses[minterCollateralIndex])
            .safeTransfer(msg.sender, collateralAmount);
    }

Duplicate of #1