An attacker can frontrun setPool() and perform a single-sided deposit into the uAD-3CRV pool to cause a DoS when Ubiquity sets the uAD-3CRV pool

Summary

The setPool() function can be frontrun in order to cause a Denial of Service by manipulating the uAD-3CRV pool's balance, given that setPool() imposes a restriction in which balances of the two tokens from the new pool must be exactly equal (a restriction that can be easily manipulated by an attacker to make it always fail by interacting with the pool).

Vulnerability Detail

Ubiquity uses a custom deployed Curve metapool (uAD-3CRV pool) in order to use it as a TWAP oracle. In order to set the pool, the setPool() in the TWAPOracleDollar3poolFacet.sol contract is used:

// TWAPOracleDollar3poolFacet.sol
function setPool(
        address _pool,
        address _curve3CRVToken1
    ) external onlyOwner {
        return LibTWAPOracle.setPool(_pool, _curve3CRVToken1);
    }

This function will internally call LibTWAPOracle's setPool() function (for simplicity, the snippet shows only the parts of the function required to understand the vulnerability):

function setPool(address _pool, address _curve3CRVToken1) internal {
        ...

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

        ...

        // ensure that pair balance is perfect
        require(_reserve0 == _reserve1, "TWAPOracle: PAIR_UNBALANCED");

                ...
    }

As we can see, the current balances of the pool will be queried and stored in the _reserve0 and _reserve1 variables when setting the pool into the protocol. After that, a perfect balance between pool’s reserve 0 and reserve 1 will be required in order for the function to execute succesfully. Here lies the vulnerability.

The requirement of balances being exactly the same (”perfect”, as mentioned in Ubiquity’s comment) makes the function susceptible of being DoS’ed. An attacker can frontrun the setPool() function and perform a single-sided deposit of 3CRV token into the uAD-3CRV pool, effectively causing an imbalance in the reserve amounts of the metapool. Only depositing a small amount such as 1 wei of 3CRV will already make the function revert, due to the requirement of the balance needing to be perfect being too strict.

Note: The deployment script found in ubiquity-dollar/packages/contracts/migrations/mainnet/Deploy001_Diamond_Dollar.s.sol also confirms that the setPool() function is indeed susceptible of frontrunning, given that the setPool() is expected to be ran as an individual transaction (this will be done after Ubiquity deploys the Curve's Dollar-3CRV metapool). The impact is even higher due to the fact that the protocol will initially not have a pool initialized by default, and will need to wait for the initial setPool() function:

// Deploy001_Diamond_Dollar.s.sol

...

/*
        TODO: uncomment when we redeploy Curve's Dollar-3CRV metapool with the new Dollar token

        TWAPOracleDollar3poolFacet twapOracleDollar3PoolFacet = TWAPOracleDollar3poolFacet(address(diamond));

        // set Curve Dollar-3CRVLP pool in the diamond storage
        twapOracleDollar3PoolFacet.setPool(
            address(curveDollarMetaPool),
            address(curveTriPoolLpToken)
        );

        */

Impact

The impact for this vulnerability is high. A malicious attacker can frontrun the setPool() function as many times as they want and imbalance the metapool without an amount as little as 1 wei of 3CRV token, effectively making the setPool() transaction always revert, preventing the protocol from being properly deployed.

If the pool is not possible of being set into the protocol, the whole protocol will be unusable, making the vulnerability be of high impact.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/facets/TWAPOracleDollar3poolFacet.sol#L22

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

Although the need for the pool to be perfectly balanced is understandable (this will make the uAD peg to 3CRV be perfect) it is recommended to not have such a restrictive check in order to be able to set the pool.

Additionally, it would be good to perform the initialization of the pool for the first time in a different manner so that frontrunning is not possible. One way would be to programatically create the pool and perform a balanced deposit, and then set the pool, all done in a single, atomic transaction.

Duplicate of #14

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