Strict check in TWAP Oracle set up can be easily griefed

Summary

A strict requirement while bootstrapping the TWAP oracle may lead to an accidental or intentional denial of service.

Vulnerability Detail

The TWAP oracle used to estimate the price of uAD relies on a Curve pool that pairs uAD with 3CRV. As part of the set up of this oracle, it is required that both assets are equal in the pool.

uint256 _reserve0 = uint112(IMetaPool(_pool).balances(0));
uint256 _reserve1 = uint112(IMetaPool(_pool).balances(1));

// ensure that there's liquidity in the pair
require(_reserve0 != 0 && _reserve1 != 0, "TWAPOracle: NO_RESERVES");
// ensure that pair balance is perfect
require(_reserve0 == _reserve1, "TWAPOracle: PAIR_UNBALANCED");

The implementation fetches both balances and compares them using a strict equality. This strictness will be difficult to hold because any slight modification in the pool could introduce an unbalance (remember we are dealing with wei amounts in tokens with 18 decimals). Operations such as add liquidity, remove liquidity or just a single swap will surely result in an unbalanced amount of reserves. So even if the pool is initially seeded with an equal amount of tokens, any operation will nullify this, causing a denial of service while setting up the TWAP oracle. This can be accidental, or intentional as a malicious actor could be purposely trying to grief the protocol.

Moreover, since the paired asset is 3CRV, which is the LP token of the 3CRV Curve Pool, this means that the paired token is not valued at $1, as it factors in the fees from the pool (see the current price at $1.03 here: https://etherscan.io/token/0x6c3F90f043a72FA612cbac8115EE7e52BDe6E490). So, in addition to the mentioned issues, it would not technically be correct to equally pair uAD to 3CRV since both token values are dissimilar.

Impact

The strict requirement will likely cause a denial of service while setting up the TWAP oracle, an issue that could also be intentionally griefed by a bad actor.

It is also incorrect to assume an equal amount of reserves of uAD and 3CRV, as the latter is not pegged to $1.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/deprecated/TWAPOracle.sol#L27-L33

Tool used

Manual Review

Recommendation

Remove the check require(_reserve0 == _reserve1, "TWAPOracle: PAIR_UNBALANCED");. If the protocol still wants to require an equal amount of assets, then remove the strict check in favor of a relaxed comparison, like asserting that the absolute difference of reserves is under a certain range.

Duplicate of #14

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