KingNFT - The formula used in getDollarPriceUsd() is incorrect

[sherlock-admin]

KingNFT

high

The formula used in getDollarPriceUsd() is incorrect

Summary

The getDollarPriceUsd() is intended to return USD price of uAD, but actually it returns the 3CRV price of per uAD. The wrong price would cause potential fund loss to users while interacting with the protocol.

Vulnerability Detail

The following PoC is built on fork of the real mainnet environment, we can see: while the average price design of twapOracleDollar3PoolFacet works, the report uAD USD price is about 0.97, but the intended result should be very close to 1.0.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;

import "./diamond/DiamondTestSetup.sol";
import "../src/dollar/libraries/Constants.sol";
import "forge-std/console2.sol";

interface ICurveFactory {
    function deploy_metapool(
        address basePool,
        string memory name,
        string memory symbol,
        address token,
        uint256 A,
        uint256 fee
    ) external returns (address);
}

interface ICurveMetapool {
    function coins(uint256) external view returns (address);

    function balances(uint256) external view returns (uint256);

    function A() external view returns (uint256);

    function get_virtual_price() external view returns (uint256);

    function add_liquidity(
        uint256[2] memory _amounts,
        uint256 _min_mint_amount,
        address _receiver
    ) external returns (uint256);
}

interface IERC20 {
    function transfer(address to, uint256 value) external returns (bool);

    function approve(address spender, uint256 value) external returns (bool);
}

contract IncorrectUbiquityDollarUSDPriceTest is DiamondTestSetup {
    uint256 constant MAINNET_BLOCK_HEIGHT = 18940000; // Jan-05-2024 08:49:47 AM +UTC
    string constant MAINNET_RPC_URL = "https://eth.ubq.fi/v1/mainnet";
    address constant CURVE_BASE_POOL =
        0xbEbc44782C7dB0a1A60Cb6fe97d0b483032FF1C7;
    IERC20 constant CURVE_3CRV_TOKEN =
        IERC20(0x6c3F90f043a72FA612cbac8115EE7e52BDe6E490);
    address constant CURVE_WHALE = 0x4486083589A063ddEF47EE2E4467B5236C508fDe;
    ICurveFactory constant CURVE_FACTORY =
        ICurveFactory(0x0959158b6040D32d04c301A72CBFD6b39E21c9AE);

    uint256 mainnetFork;
    ICurveMetapool metapool;

    function setUp() public override {
        mainnetFork = vm.createSelectFork(
            MAINNET_RPC_URL,
            MAINNET_BLOCK_HEIGHT
        );

        super.setUp();

        // deploy with the same parameters as old uAD3CRV pool, reference:
        // https://etherscan.io/tx/0x9e08f16634114d72a501d06b97057a878af124b4845b7f299a9502ef3139847c
        metapool = ICurveMetapool(
            CURVE_FACTORY.deploy_metapool(
                CURVE_BASE_POOL,
                dollarToken.name(),
                dollarToken.symbol(),
                address(dollarToken),
                10,
                4000000
            )
        );
        assertEq(address(dollarToken), metapool.coins(0));
        assertEq(address(CURVE_3CRV_TOKEN), metapool.coins(1));

        vm.prank(CURVE_WHALE);
        CURVE_3CRV_TOKEN.transfer(admin, 200_000e18);
        vm.prank(admin);
        dollarToken.mint(admin, 200_000e18);

        vm.startPrank(admin);
        dollarToken.approve(address(metapool), type(uint256).max);
        CURVE_3CRV_TOKEN.approve(address(metapool), type(uint256).max);
        vm.stopPrank();
    }

    function testIncorrectUbiquityDollarUSDPrice() public {
        // 1. add the initial liquidity
        uint256[2] memory amounts = [uint256(100_000e18), uint256(100_000e18)];
        vm.prank(admin);
        metapool.add_liquidity(amounts, 0, admin);
        assertEq(100_000e18, metapool.balances(0));
        assertEq(100_000e18, metapool.balances(1));

        // 2. set pool to diamond, the initial price is the intended $1
        vm.prank(owner);
        twapOracleDollar3PoolFacet.setPool(
            address(metapool),
            address(CURVE_3CRV_TOKEN)
        );
        twapOracleDollar3PoolFacet.update();
        uint256 price = ubiquityPoolFacet.getDollarPriceUsd();
        assertEq(1e6, price);

        // 3. step to next block and add some new negligible liquidity to make average price work
        vm.warp(block.timestamp + 12);
        vm.roll(block.number + 1);
        vm.prank(admin);
        uint256[2] memory amountsNegligible = [uint256(1e12), uint256(1e12)];
        metapool.add_liquidity(amountsNegligible, 0, admin);
        twapOracleDollar3PoolFacet.update();

        // 4. now, the price becomes incorrect
        price = ubiquityPoolFacet.getDollarPriceUsd();
        uint256 maxDiff = 0.001e6;
        assertApproxEqAbs(0.974e6, price, maxDiff);

        // 5. the reason is that 3CRV's virtual price is not counted in, the correct formula is:
        uint256 virtualPrice = ICurveMetapool(CURVE_BASE_POOL).get_virtual_price();
        price = price * virtualPrice / 1e18;
        assertApproxEqAbs(1.002e6, price, maxDiff);
        // note: here it's 1.002 rather than 1.000, the reason is also about 3CRV's virtual price
        // due to this, uAD3CRV metapool has 100K $USD value of uAD VS 102.8K $USD value of 3CRV,
        // the unbalance of USD values causes the $0.002 drift of uAD price
    }
}

The test log:

2023-12-ubiquity\ubiquity-dollar\packages\contracts> forge test --match-contract IncorrectUbiquityDollarUSDPriceTest -vv
[⠔] Compiling...
[⠘] Compiling 1 files with 0.8.19
[⠃] Solc 0.8.19 finished in 17.26sCompiler run successful!
[⠊] Solc 0.8.19 finished in 17.26s

Running 1 test for test/IncorrectUbiquityDollarUSDPrice.t.sol:IncorrectUbiquityDollarUSDPriceTest
[PASS] testIncorrectUbiquityDollarUSDPrice() (gas: 731546)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.13s

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

The wrong report uAD price would cause users suffering fund loss while interact with the protocol, for example:
if the current correct uAD price is $1.05, but the report price is $1.0, a user calls UbiquityPoolFacet.redeemDollar() to redeem 10k uAD which is worth 10.5k USD, but only get 10k USD value of LUSD back, a 500 USD loss occurs.

Code Snippet

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

Tool used

Manual Review

Recommendation

see PoC for the correct formula

Duplicate of #59

[sherlock-admin2]

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

auditsea commented:

USD price of Ubiquity Dollar is determined by underlying LP price

[sherlock-admin2]

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

auditsea commented:

USD price of Ubiquity Dollar is determined by underlying LP price