V3Oracle susceptible to price manipulation

mediumCode4rena

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Oracle.sol#L120

Vulnerability details

Impact

V3Oracle::getValue() is used to calculate the value of a position. The value is the product of the oracle price * the amounts held in the position. Price manipulation is prevented by checking for differences between Chainlink oracle and Uniswap TWAP.

However the amounts (amount0,amount1) of the tokens in the position are calculated based on the current pool price (pool.spot0()), which means they can be manipulated. And since the value of the total position is calculated from amount0,amount1 it can be manipulated as well.

Proof of Concept

Invoking V3Oracle::getValue() first calls the getPositionBreakdown() to calculate the amount0 and amount1 of the tokens in the position based on spot price:

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Oracle.sol#L102

 (address token0, address token1, uint24 fee,, uint256 amount0, uint256 amount1, uint256 fees0, uint256 fees1) =
            getPositionBreakdown(tokenId);

Under the hood this calls _initializeState() which get's the current price in the pool: https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Oracle.sol#L395

(state.sqrtPriceX96, state.tick,,,,,) = state.pool.slot0();

Based on this value the amount0 & amount1 (returned from getPositionBreakdown() are deduced:

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Oracle.sol#L426

function _getAmounts(PositionState memory state)
        internal
        view
        returns (uint256 amount0, uint256 amount1, uint128 fees0, uint128 fees1)
    {
        if (state.liquidity > 0) {
       ....
            (amount0, amount1) = LiquidityAmounts.getAmountsForLiquidity(
                state.sqrtPriceX96, state.sqrtPriceX96Lower, state.sqrtPriceX96Upper, state.liquidity
            );
        }
       ....
    }

After that the prices are fetched from Uniswap & Chainlink and compared.

 (price0X96, cachedChainlinkReferencePriceX96) =
            _getReferenceTokenPriceX96(token0, cachedChainlinkReferencePriceX96);
 (price1X96, cachedChainlinkReferencePriceX96) =
            _getReferenceTokenPriceX96(token1, cachedChainlinkReferencePriceX96);

Finally the value of the positions tokens and fees are calculated in the following formula:

value = (price0X96 * (amount0 + fees0) / Q96 + price1X96 * (amount1 + fees1) / Q96) * Q96 / priceTokenX96;
feeValue = (price0X96 * fees0 / Q96 + price1X96 * fees1 / Q96) * Q96 / priceTokenX96;
price0X96 = price0X96 * Q96 / priceTokenX96;
price1X96 = price1X96 * Q96 / priceTokenX96;

Basically the position value is a product of 2 parameters price0X96/price1X96 & amount0/amount1:

• price0X96/price1X96 - those are the prices derived from the oracles - they are validated and cannot be manipulated
• amount0/amount1 - those are calculated based on the spot price and can be manipulated

Since amount0 & amount1 can be increased/decreased if a malicious user decides to distort the pool price in the current block (through a flash loan for example), this will directly impact the calculated value, even though the price itself cannot be manipulated since it is protected against manipulation

The check in the end _checkPoolPrice() only verifies that the price from the oracles is in the the acceptable ranges. This however does not safeguard the value calculation, which as explained above also includes the amounts parameters.

It should be noted that _checkPoolPrice uses the uniswap TWAP price for comparison, which is the price over an extended period of time making it very hard to manipulate in a single block. And exactly this property of the TWAP price can allow an attacker to manipulate the spot price significantly, without affecting the TWAP much, which means the price difference won't change much and _checkPoolPrice will pass.

A short example:

• The V3Oracle has been configured to use a TWAP duration of 1 hour
• The TWAP price reported for the last hour is 4000 USDC for 1 WETH
• Bob takes a flash loan to distort the spot price heavily and call in the same transaction borrow | repay on a V3Vault (which call V3Oracle.getValue())
• Because of the price manipulation amount1 & amoun0 are heavily inflated/deflated
• However this changes the TWAP value only a little (if at all), so the price validation passes
• The position value is calculated by multiplying the stable oracle price by the heavily manipulated amounts
• User repay/borrows at favorable conditions

Tools Used

Manual Review

Recommended Mitigation Steps

Consider calculating amount0 & amount1 based on the oracle price and not on the spot price taken from slot0(). This way the above exploit will be mitigated

Assessed type

Uniswap