sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

infect3d - LibTWAPOracle.setPool can be DoS'd by sending 1 wei to imbalance pools reserves #176

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 8 months ago

infect3d

medium

LibTWAPOracle.setPool can be DoS'd by sending 1 wei to imbalance pools reserves

Summary

It is cheap and easy to DoS/prevent the setting of a pool as an oracle

Vulnerability Detail

the setPool function is used to set a Curve MetaPool as the TWAP oracle reference. There's multiple sanity checks performed inside, one of them ensuring that the reserve of both tokens is the same. It make the call vulnerable to DoS front-running by sending 1 wei of any of both tokens to imbalance the pool and make the call revert. Also, if you check the different MetaPools, you'll see that reserves aren't balanced.

Impact

Not allowing to set up the oracle

Code Snippet

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

File: src\dollar\libraries\LibTWAPOracle.sol
32:     function setPool(address _pool, address _curve3CRVToken1) internal {
33:         require(
34:             IMetaPool(_pool).coins(0) ==
35:                 LibAppStorage.appStorage().dollarTokenAddress,
36:             "TWAPOracle: FIRST_COIN_NOT_DOLLAR"
37:         );
38:         TWAPOracleStorage storage ts = twapOracleStorage();
39: 
40:         // coin at index 0 is Ubiquity Dollar and index 1 is 3CRV
41:         require(
42:             IMetaPool(_pool).coins(1) == _curve3CRVToken1,
43:             "TWAPOracle: COIN_ORDER_MISMATCH"
44:         );
45: 
46:         uint256 _reserve0 = uint112(IMetaPool(_pool).balances(0));
47:         uint256 _reserve1 = uint112(IMetaPool(_pool).balances(1));
48:
49:         // ensure that there's liquidity in the pair
50:         require(_reserve0 != 0 && _reserve1 != 0, "TWAPOracle: NO_RESERVES");
51:         // ensure that pair balance is perfect
52: @>      require(_reserve0 == _reserve1, "TWAPOracle: PAIR_UNBALANCED"); 
53:         ts.priceCumulativeLast = IMetaPool(_pool).get_price_cumulative_last()
54:         ts.pricesBlockTimestampLast = IMetaPool(_pool).block_timestamp_last();
55:         ts.pool = _pool;
56:         // dollar token is inside the diamond
57:         ts.token1 = _curve3CRVToken1;
58:         ts.price0Average = 1 ether;
59:         ts.price1Average = 1 ether;
60:     }

Tool used

Manual Review

Recommendation

There is no reason to ensure balances are perfect in the pool, as this is only true at the very beginning of the existence of the pool. Any first trade will imbalance the reserves.

Duplicate of #14

sherlock-admin2 commented 7 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

sherlock-admin2 commented 7 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