FloorDAO / floor-v2

Floor aims to create a fully onchain governance mechanism for sweeping and deploying NFTs to profitable NFT-Fi strategies as well as seeding liquidity for its own NFT-Fi products.
https://floor.xyz
2 stars 0 forks source link

[UVP-03M] Decoding Loss of Precision #107

Closed tomwade closed 12 months ago

tomwade commented 1 year ago

UVP-03M: Decoding Loss of Precision

Type Severity Location
Mathematical Operations UniswapV3PricingExecutor.sol:L211, L213

Description:

The UniswapV3PricingExecutor::_decodeSqrtPriceX96 function will attempt to decode the Uniswap V3 encoded price to the pair's price, however, it does so without properly accommodating for mathematical inaccuracies.

Impact:

Price calculations that end out-of-bounds could result in improper zero-price assessments.

Example:

function _decodeSqrtPriceX96(address underlying, uint underlyingDecimalsScaler, uint sqrtPriceX96) private pure returns (uint price) {
    if (uint160(underlying) < uint160(WETH)) {
        price = FullMath.mulDiv(sqrtPriceX96, sqrtPriceX96, uint(2 ** (96 * 2)) / 1e18) / underlyingDecimalsScaler;
    } else {
        price = FullMath.mulDiv(sqrtPriceX96, sqrtPriceX96, uint(2 ** (96 * 2)) / (1e18 * underlyingDecimalsScaler));
        if (price == 0) return 1e36;
        price = 1e36 / price;
    }

    if (price > 1e36) price = 1e36;
    else if (price == 0) price = 1;
}

Recommendation:

We advise the code to instead multiply sqrtPriceX96 by itself, divide by the token1 decimals, multiply by the token0 decimals, and finally divide by the 2 ** 192 value. This order of operations will guarantee maximum accuracy of the price calculated whilst ensuring that it remains within safe bounds by balancing the multiplication and division operations.

tomwade commented 1 year ago

This was taken from the Euler Risk Manager contract: https://github.com/euler-xyz/euler-contracts/blob/master/contracts/modules/RiskManager.sol

I'll look at reviewing this issue to see if I can create a test suite that breaks the current flow and would be resolved by the change in formula but will make this a lower priority.