TWAPOracle price are calculated on very small windows

Summary

The LibTWAPOracle library responsible for calculating the price of Ubiquity dollar based on the 3CRV/Ubiquity dollar metapool uses time window that start at the previous update and end on the current block. This time window can be as small as a single block and is not suitable for a TWAP price calculation.

Vulnerability Detail

The function to update the quote prices of the library uses the time window blockTimestamp - ts.pricesBlockTimestampLast to calculate the TWAP before updating ts.pricesBlockTimestampLast to blockTimestamp:

    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;
        }
    }

blockTimestamp is the latest update timestamp on the metapool:

    function currentCumulativePrices()
        internal
        view
        returns (uint256[2] memory priceCumulative, uint256 blockTimestamp)
    {
        address metapool = twapOracleStorage().pool;
        priceCumulative = IMetaPool(metapool).get_price_cumulative_last();
        blockTimestamp = IMetaPool(metapool).block_timestamp_last();
    }

@internal
def _update():
    """
    Commits pre-change balances for the previous block
    Can be used to compare against current values for flash loan checks
    """
    elapsed_time: uint256 = block.timestamp - self.block_timestamp_last
    if elapsed_time > 0:
        for i in range(N_COINS):
            _balance: uint256 = self.balances[i]
            self.price_cumulative_last[i] += _balance * elapsed_time
            self.previous_balances[i] = _balance
        self.block_timestamp_last = block.timestamp

If the metapool is updated on block x and x+1 and the TWAP oracle is updated on the same blocks, then the time window for TWAP calculation is only 1 block.

The update can be triggered externally with no restriction by anyone on the metapool (via any action that updates the balances) and on the TWAPOracle via TWAPOracleDollar3poolFacet.update().

Impact

The TWAP price for Ubiquity dollar relies on a very short window and is extremely easy to manipulate.

This TWAP price is used to determine whether minting/redeeming Ubiquity dollar is allowed in LibUbiquityPool.sol. These minting/redeeming functions are core to the stability of the Ubiquity dollar and if not properly protected will make the coin unstable.

Code Snippet

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

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

https://etherscan.io/address/0x20955CB69Ae1515962177D164dfC9522feef567E#code

Tool used

Manual Review

Recommendation

Use a longer time window (e.g. a week) to calculate the TWAP price for Ubiquity dollar on the metapool.

Duplicate of #20

sherlock-admin2 commented 6 months ago

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

auditsea commented:

The issue describes about TWAP can be manipulated because update function can be called anytime and by anyone, thus TWAP period can be as short as 1 block. It seems like a valid issue but after caeful consideration, it's noticed that the TWAP issue does not come from its period but the logic itself is incorrect, thus marking this as Invalid