Users could mint and redeem Ubiquity Dollar with a more favorable Dollar price

Summary

Upon establishing a new MetaPool, the Ubiquity Dollar's price is configured to 1 ether. In the event that users execute either the mintDollar or redeemDollar functions within the same block as the setPool operation, they have the opportunity to leverage the 1 ether price for potential profits.

Vulnerability Detail

In the provided code snippet, the function setPool is utilized to set a new MetaPool by the owner. And, it establishes the price of Ubiquity Dollar to the default value of 1 ether.

    function setPool(address _pool, address _curve3CRVToken1) internal {
        require(
            IMetaPool(_pool).coins(0) ==
                LibAppStorage.appStorage().dollarTokenAddress,
            "TWAPOracle: FIRST_COIN_NOT_DOLLAR"
        );
        TWAPOracleStorage storage ts = twapOracleStorage();

        // coin at index 0 is Ubiquity Dollar and index 1 is 3CRV
        require(
            IMetaPool(_pool).coins(1) == _curve3CRVToken1,
            "TWAPOracle: COIN_ORDER_MISMATCH"
        );

        uint256 _reserve0 = uint112(IMetaPool(_pool).balances(0));
        uint256 _reserve1 = uint112(IMetaPool(_pool).balances(1));

        // ensure that there's liquidity in the pair
        require(_reserve0 != 0 && _reserve1 != 0, "TWAPOracle: NO_RESERVES");
        // ensure that pair balance is perfect
        require(_reserve0 == _reserve1, "TWAPOracle: PAIR_UNBALANCED");
        ts.priceCumulativeLast = IMetaPool(_pool).get_price_cumulative_last();
        ts.pricesBlockTimestampLast = IMetaPool(_pool).block_timestamp_last();
        ts.pool = _pool;
        // dollar token is inside the diamond
        ts.token1 = _curve3CRVToken1;
//@audit the price is set to 1 ether, rather than the actual price
        ts.price0Average = 1 ether;
        ts.price1Average = 1 ether;
    }

Let's examine the logic of price updates within the update() function. It's essential to note that the price is eligible for an update only when blockTimestamp > ts.pricesBlockTimestampLast.

    function update() internal {
        TWAPOracleStorage storage ts = twapOracleStorage();
        (
            uint256[2] memory priceCumulative,
            uint256 blockTimestamp
        ) = currentCumulativePrices();
        if (blockTimestamp - ts.pricesBlockTimestampLast > 0) {
            // get the balances between now and the last price cumulative snapshot
            uint256[2] memory twapBalances = IMetaPool(ts.pool)
                .get_twap_balances(
                    ts.priceCumulativeLast,
                    priceCumulative,
                    blockTimestamp - ts.pricesBlockTimestampLast
                );

            // price to exchange amountIn Ubiquity Dollar to 3CRV based on TWAP
            ts.price0Average = IMetaPool(ts.pool).get_dy(
                0,
                1,
                1 ether,
                twapBalances
            );

            // price to exchange amountIn 3CRV to Ubiquity Dollar based on TWAP
            ts.price1Average = IMetaPool(ts.pool).get_dy(
                1,
                0,
                1 ether,
                twapBalances
            );
            // we update the priceCumulative
            ts.priceCumulativeLast = priceCumulative;
            ts.pricesBlockTimestampLast = blockTimestamp;
        }
    }

Hence, should an attacker execute the mintDollar immediately after the establishment of a new MetaPool (within the same block and timestamp), the Ubiquity Dollar's price will promptly revert to 1 ether.

In the event that 1 ether happens to represent a lower price and exceeds the mintPriceThreshold, the attacker stands to obtain more collateral than intended. This scenario also holds true for the redeemDollar function when 1 ether exceeds the current price.

    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
//@audit getDollarPriceUsd is 1 ether if this is invoked in the same block with setPool
        require(
            getDollarPriceUsd() >= poolStorage.mintPriceThreshold,
            "Dollar price too low"
        );

Impact

Users could mint and redeem Ubiquity Dollar with a more favorable Dollar price and profit from that.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibTWAPOracle.sol#L31-L59 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibTWAPOracle.sol#L68-L102 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L326-L349

Tool used

Manual Review

Recommendation

It's recommended to set the price0Average and price1Average to the correct value when setting new Pool.

Duplicate of #20

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

In the beginning, it's guaranteed that token price is $1