Any collateral index can be passed by the user which can cause loss of funds

Summary

A user can call mintDollar and redeemDollar with inputs for collateral index different for both mintDollar and redeemDollar which can lead loss of collateral tokens from the contract which are transferred from other users.

Vulnerability Detail

Following is the mintDollar function in UbiquityPoolFacet.sol

function mintDollar(
        uint256 collateralIndex,
        uint256 dollarAmount,
        uint256 dollarOutMin,
        uint256 maxCollateralIn
    ) external returns (uint256 totalDollarMint, uint256 collateralNeeded) {
        return
            LibUbiquityPool.mintDollar(
                collateralIndex,
                dollarAmount,
                dollarOutMin,
                maxCollateralIn
            );
    }

Which calls the mintDollar in LibUbiquityPool.sol which is as follows

function mintDollar(
        uint256 collateralIndex,
        uint256 dollarAmount,
        uint256 dollarOutMin,
        uint256 maxCollateralIn
    )
        internal
        collateralEnabled(collateralIndex)
        returns (uint256 totalDollarMint, uint256 collateralNeeded)
    {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        require(
            poolStorage.isMintPaused[collateralIndex] == false,
            "Minting is paused"
        );

        // update Dollar price from Curve's Dollar Metapool
        LibTWAPOracle.update();
        // prevent unnecessary mints
        require(
            getDollarPriceUsd() >= poolStorage.mintPriceThreshold,
            "Dollar price too low"
        );

        // update collateral price
        updateChainLinkCollateralPrice(collateralIndex);

        // get amount of collateral for minting Dollars
        collateralNeeded = getDollarInCollateral(collateralIndex, dollarAmount);

        // subtract the minting fee
        totalDollarMint = dollarAmount
            .mul(
                UBIQUITY_POOL_PRICE_PRECISION.sub(
                    poolStorage.mintingFee[collateralIndex]
                )
            )
            .div(UBIQUITY_POOL_PRICE_PRECISION);

        // check slippages
        require((totalDollarMint >= dollarOutMin), "Dollar slippage");
        require((collateralNeeded <= maxCollateralIn), "Collateral slippage");

        // check the pool ceiling
        require(
            freeCollateralBalance(collateralIndex).add(collateralNeeded) <=
                poolStorage.poolCeilings[collateralIndex],
            "Pool ceiling"
        );

        // take collateral first
        IERC20(poolStorage.collateralAddresses[collateralIndex])
            .safeTransferFrom(msg.sender, address(this), collateralNeeded);

        // mint Dollars
        IERC20Ubiquity ubiquityDollarToken = IERC20Ubiquity(
            LibAppStorage.appStorage().dollarTokenAddress
        );
        ubiquityDollarToken.mint(msg.sender, totalDollarMint);
    }

main issue is in the function which is used to collateral needed in order to mint dollar

 // get amount of collateral for minting Dollars
        collateralNeeded = getDollarInCollateral(collateralIndex, dollarAmount);

getDollarInCollateralIndex function is as follows

 function getDollarInCollateral(
        uint256 collateralIndex,
        uint256 dollarAmount
    ) internal view returns (uint256) {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();
        return
            dollarAmount
                .mul(UBIQUITY_POOL_PRICE_PRECISION)
                .div(10 ** poolStorage.missingDecimals[collateralIndex])
                .div(poolStorage.collateralPrices[collateralIndex]);
    }

So lets assune missing decimals = 0 so the collateral needed will be calculated as follows

dollarAmount
                .mul(UBIQUITY_POOL_PRICE_PRECISION)
                .div(poolStorage.collateralPrices[collateralIndex]);

Lets assume collateral price = 20 dollars i.e poolStorage.collateralPrices[collateralIndex] = 20*10**6 = 2e7 and if dollar amount = 19 then collateral needed will be equal to 19.mul(e6).div(20e6) = 0 So the user can mint dollar tokens for free. Not only this now user has 19 dollar tokens - minting fees so lets assume user got 18 dollar tokens and if he calls the redeemDollar function which is as follows

    /// @inheritdoc IUbiquityPool
    function redeemDollar(
        uint256 collateralIndex,
        uint256 dollarAmount,
        uint256 collateralOutMin
    ) external returns (uint256 collateralOut) {
        return
            LibUbiquityPool.redeemDollar(
                collateralIndex,
                dollarAmount,
                collateralOutMin
            );
    }

As you can see there is no check on collateralIndex which asserts that the collateral index should be same as the one with which dollar has been minted. So the user can call the collateral index which has price less than the collateralIndex with which he minted the dollar For example now lets assume the price of new collateral = 10 dollars i.e 10e6 So now the redeemFunction is as follows

function redeemDollar(
        uint256 collateralIndex,
        uint256 dollarAmount,
        uint256 collateralOutMin
    )
        internal
        collateralEnabled(collateralIndex)
        returns (uint256 collateralOut)
    {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

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

        // update Dollar price from Curve's Dollar Metapool
        LibTWAPOracle.update();
        // prevent unnecessary redemptions that could adversely affect the Dollar price
        require(
            getDollarPriceUsd() <= poolStorage.redeemPriceThreshold,
            "Dollar price too high"
        );

        uint256 dollarAfterFee = dollarAmount
            .mul(
                UBIQUITY_POOL_PRICE_PRECISION.sub(
                    poolStorage.redemptionFee[collateralIndex]
                )
            )
            .div(UBIQUITY_POOL_PRICE_PRECISION);

        // update collateral price
        updateChainLinkCollateralPrice(collateralIndex);

        // get collateral output for incoming Dollars
        collateralOut = getDollarInCollateral(collateralIndex, dollarAfterFee);

        // checks
        require(
            collateralOut <=
                (IERC20(poolStorage.collateralAddresses[collateralIndex]))
                    .balanceOf(address(this))
                    .sub(poolStorage.unclaimedPoolCollateral[collateralIndex]),
            "Insufficient pool collateral"
        );
        require(collateralOut >= collateralOutMin, "Collateral slippage");

        // account for the redeem delay
        poolStorage.redeemCollateralBalances[msg.sender][
            collateralIndex
        ] = poolStorage
        .redeemCollateralBalances[msg.sender][collateralIndex].add(
                collateralOut
            );
        poolStorage.unclaimedPoolCollateral[collateralIndex] = poolStorage
            .unclaimedPoolCollateral[collateralIndex]
            .add(collateralOut);

        poolStorage.lastRedeemedBlock[msg.sender] = block.number;

        // burn Dollars
        IERC20Ubiquity ubiquityDollarToken = IERC20Ubiquity(
            LibAppStorage.appStorage().dollarTokenAddress
        );
        ubiquityDollarToken.burnFrom(msg.sender, dollarAmount);
    }

So now the dollar amount passed is reduced by the redemption fees that the user has to pay lets assume after redemption fees is applied the the token left = 16 dollar tokes Now the amount of collateral the user would get after redeeming the dollar tokens is calculated as follows

// get collateral output for incoming Dollars
        collateralOut = getDollarInCollateral(collateralIndex, dollarAfterFee);

GetDollarIncollateral is same as before ,so now we have assumed the price of collateral = 10 dollars i.e 10e6 so the value returned which is collateralOut = 16.mul(e6).div(10e6) = 1 collateral token So from the above it can be seen not only a user can get dollar tokens for free, he can even get a collateral tokens for free. So whenever a there are two or more collaterals with different prices this situation can occur and as the sponsers are planning to use this currently for LUSD and DAI ,price of LUSD is more than that of DAI ,so this attack is feasible.

Impact

Can cause minting of dollar tokens for free and also can cause loss of collateral from the contract which is transferred by other genuine users.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/facets/UbiquityPoolFacet.sol#L92C1-L106C1 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/facets/UbiquityPoolFacet.sol#L77 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L284

Tool used

Manual Review

Recommendation

Add some checks to enforce that the collateralIndexs are same in redeem and mint dollar functions

Duplicate of #7

sherlock-audit / 2023-12-ubiquity-judging

Varun_05 - Any collateral index can be passed by the user which can cause loss of funds #189