Curve pool may not be set as oracle, because it perfect 1:1 ratio of uAD, which is hardly possible and anyone can break it depositing/swapping 1 wei

Summary

Protocol sets curve uAD/3CRV metapool as an oracle pool. There is function LibTWAPOracle::setPool, which set the pool for ubiquity pool, but there is one validation, which is hardly achievable require(_reserve0 != 0 && _reserve1 != 0, "TWAPOracle: NO_RESERVES");

Vulnerability Detail

Curve metapools are pools between 3pool (DAI, USDC, USDT) and another stablecoin pair. The pool liquidity aims to be balanced between the pair, but it is almost impossible to have exact 1:1 allocation of funds in metapool. Here is an example for quite stable pool USD Metapool: Liquity:

balances[0] = 1744991529279388548540004
balances[1] = 12642007799752756617751200

For each stable metapool that you check, there won't be 1:1 ratio between two assets, which means that following the current implementation, code cannot be deployed. Tests are working, because mocked metapool doesn't really act as a real one.

Coded PoC:

// Location `contracts/src/dollar/mocks/MockMetaPool.sol`
function add_liquidity(
        uint256[2] memory _amounts,
        uint256 _min_mint_amount,
        address _receiver
    ) external returns (uint256 result) {
        mint(
            _receiver,
            _min_mint_amount == 0
                ? _amounts[0] > _amounts[1] ? _amounts[0] : _amounts[1]
                : _min_mint_amount
        );
+        balances[0] += _amounts[0];
+        balances[1] += _amounts[1];
        return result;
    }

// Location `contracts/test/diamond/facets/TWAPOracleFacet.t.sol`
function setUp() public override {
        super.setUp();

+        vm.startPrank(admin);
+        dollarToken.mint(address(this), 100);
+        vm.stopPrank();

        metaPoolAddress = address(
            new MockMetaPool(address(dollarToken), curve3CRVTokenAddress)
        );

+        // Front-run transaction, which cost low and stop TWAP pool from being set
+        uint256[2] memory arr = [uint256(1), uint256(0)];
+        MockMetaPool(metaPoolAddress).add_liquidity(arr, 0, address(this));

        vm.prank(owner);
+        vm.expectRevert();
        twapOracleDollar3PoolFacet.setPool(
            metaPoolAddress,
            curve3CRVTokenAddress
        );
    }

+    function testSetUpNotPossibleWhenMaliciousUserDeposit1weiInMetaPool()
+        public
+    {}

Impact

Impossibility for the protocol to set a metapool.
Need of new implementation of LibTWAPOracle
Resources for the protocol

Code Snippet

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

Tool used

Manual Review

Recommendation

If you want to remain the check implement a tolerance in % , which may be the difference between liquidity of two assets Example of 5% tolerance:

// Ensure that pair balance is within 5% tolerance
uint256 tolerance = (_reserve0 * 5) / 100;
require(
   _reserve0 >= _reserve1 - tolerance && _reserve0 <= _reserve1 + tolerance,
   "TWAPOracle: PAIR_UNBALANCED"
);

Duplicate of #14

sherlock-admin2 commented 9 months ago

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

auditsea commented:

The issue describes about DOSing setPool function by manipulating the Curve pool, but it's assumed that the Curve pool deployment, LP deposit, and setPool will be handled in one tx using multicall structure