search

code-423n4 / 2021-09-wildcredit-findings

0 stars 0 forks source link

UniswapV3Helper: getSqrtPriceX96() doesn't work for tokens with non-18 decimals #60

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

hickuphh3

Vulnerability details

Impact

The getSqrtPriceX96() will return incorrect values for pairs comprising of non-18 decimals. This affects the amounts calculated for a position.

Proof of Concept

Let us take the ETH-WBTC pair as an example. Note that WBTC has 8 decimals, and is an active lending pair on the existing implementation. At the time of writing, calling the tokenPrice on the controller returns the following values:

WBTC: 42343228224963937352954

ETH: 2910266295099999997089

The UniV3 WBTC-ETH pool returns a sqrtPriceX96 of 30222035838195858377447891771438698. It has token0 as WBTC and token1 as WETH. However, calling the getSqrtPriceX96() with the prices mentioned above returns a value of 4141069412361645843299412392.

This difference in behaviour is caused by Uniswap V3 deriving the sqrtPrice from token amounts, whereas the sqrtPrice here is being derived from oracle prices.

Recommended Mitigation Steps

A sample implementation is provided below. It assumes that all token prices returned are normalized to precision (1e18).

function getSqrtPriceX96(
    uint _price0,
    uint _price1,
    uint _decimals0,
    uint _decimals1
) public pure returns(uint) {
    _price0 = _price0 * 1e18 / 10 ** _decimals0;
    _price1 = _price1 * 1e18 / 10 ** _decimals1;
  uint result = (_price0 << 192) / _price1 ;
  result = _sqrt(result);
    return result * 1e18 / 10 ** _decimals0 * 1e18 / 10 ** _decimals1;
}
talegift commented 3 years ago

This case can never happen. This function is always called with TWAP USD values of tokens with 18 decimals.

See here: https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/UniswapV3Helper.sol#L70 https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/LendingPair.sol#L452 https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/LendingPair.sol#L671 https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/LendingPair.sol#L340

In all cases, the source of prices is LendingController.tokenPrices which itself is loading them from here: https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/UniswapV3Oracle.sol#L84

Decimal conversion is therefore already performed much earlier in UniswapV3Oracle.tokenPrice

ghoul-sol commented 3 years ago

per sponsor comment, invalid