Skip to content
This repository has been archived by the owner on Jul 14, 2024. It is now read-only.

0xpiken - Incorrect value was used to represent USD price of Ubiquity Dollar #30

Closed
sherlock-admin2 opened this issue Jan 10, 2024 · 2 comments
Labels
Non-Reward This issue will not receive a payout

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Jan 10, 2024

0xpiken

medium

Incorrect value was used to represent USD price of Ubiquity Dollar

Summary

UbiquityPoolFacet#getDollarPriceUsd() is supposed to return USD value of Ubiquity Dollar according to its doc:

Returns Ubiquity Dollar token USD price (1e6 precision) from Curve Metapool (Ubiquity Dollar, Curve Tri-Pool LP)

However, it's actually returning the price in 3CRV of the Ubiquity Dollar instead.

Vulnerability Detail

Ubiquity has a mechanism to limit Ubiquity Dollar minting and redeeming:

  • minting is not allowed when price of Ubiquity Dollar is below mintPriceThreshold
345:        // prevent unnecessary mints
346:        require(
347:            getDollarPriceUsd() >= poolStorage.mintPriceThreshold,
348:            "Dollar price too low"
349:        );
  • redeeming is not allowed when price of Ubiquity Dollar is above redeemPriceThreshold
417:        // prevent unnecessary redemptions that could adversely affect the Dollar price
418:        require(
419:            getDollarPriceUsd() <= poolStorage.redeemPriceThreshold,
420:            "Dollar price too high"
421:        );

mintPriceThreshold and redeemPriceThreshold represent price thresholds measured in USD value with 6 decimal places:

64:        // 1010000 = $1.01
65:        uint256 mintPriceThreshold;
66:        // 990000 = $0.99
67:        uint256 redeemPriceThreshold;

From above we can see, the returning value of getDollarPriceUsd() must represent USD price of Ubiquity Dollar token.

Let's take a look at how getDollarPriceUsd() works:

  • The return value comes from LibTWAPOracle.getTwapPrice() (decimals is scaled from 18 to 6):
300:    function getDollarPriceUsd()
301:        internal
302:        view
303:        returns (uint256 dollarPriceUsd)
304:    {
305:        // get Dollar price from Curve Metapool (18 decimals)
306:        uint256 dollarPriceUsdD18 = LibTWAPOracle.getTwapPrice();
307:        // convert to 6 decimals
308:        dollarPriceUsd = dollarPriceUsdD18
309:            .mul(UBIQUITY_POOL_PRICE_PRECISION)
310:            .div(1e18);
311:    }
  • getTwapPrice() return the value of ts.price0Average:
159:    function getTwapPrice() internal view returns (uint256) {
160:        return
161:            LibTWAPOracle.consult(
162:                LibAppStorage.appStorage().dollarTokenAddress
163:            );
164:    }
111:    function consult(address token) internal view returns (uint256 amountOut) {
112:        TWAPOracleStorage memory ts = twapOracleStorage();
113:
114:        if (token == LibAppStorage.appStorage().dollarTokenAddress) {
115:            // price to exchange 1 Ubiquity Dollar to 3CRV based on TWAP
116:            amountOut = ts.price0Average;
117:        } else {
118:            require(token == ts.token1, "TWAPOracle: INVALID_TOKEN");
119:            // price to exchange 1 3CRV to Ubiquity Dollar based on TWAP
120:            amountOut = ts.price1Average;
121:        }
122:    }

From Line 115 we can see that ts.price0Average is price to exchange 1 Ubiquity Dollar to 3CRV based on TWAP, which is not relevant to USD value.

ts.price0Average is updated by calling LibTWAPOracle#update(), It's clear that ts.price0Average represent how many 3CRV tokens needed for exchanging / swapping 1e18 Ubiquity Dollar:

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

Since returned amount of 3CRV token can not represent USD value of Ubiquity Dollar, mintDollar() and redeemDollar() can not work as expected.

Impact

mintDollar() and redeemDollar() may not work as expected

Code Snippet

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

Tool used

Manual Review

Recommendation

getDollarPriceUsd() should return the real USD value of Ubiquity Dollar. IMetaPool #get_dy_underlying() could be helpful in achieving this.

Duplicate of #59

@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Jan 14, 2024
@sherlock-admin2
Copy link
Contributor Author

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

auditsea commented:

Using LP price is more accurate than using price of one of underlying tokens

@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label and removed Excluded Excluded by the judge without consulting the protocol or the senior labels Jan 16, 2024
@sherlock-admin2
Copy link
Contributor Author

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

auditsea commented:

Using LP price is more accurate than using price of one of underlying tokens

@sherlock-admin2 sherlock-admin2 changed the title Salty Saffron Pheasant - Incorrect value was used to represent USD price of Ubiquity Dollar 0xpiken - Incorrect value was used to represent USD price of Ubiquity Dollar Jan 24, 2024
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Jan 24, 2024
@Czar102 Czar102 removed the High A valid High severity issue label Feb 19, 2024
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Reward A payout will be made for this issue labels Feb 19, 2024
@sherlock-admin2 sherlock-admin2 removed the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Feb 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

3 participants