M-25 Unmitigated

[c4-bot-10]

Lines of code

https://github.com/revert-finance/lend/blob/audit/src/V3Oracle.sol?plain=1#L138

Vulnerability details

C4 issue

M-25: Asymmetric calculation of price difference

Comments

V3Oracle compares the value difference between two prices to ensure that a token's price is valid. Unfortunately, the formula used to compare both prices is mathematically incorrect. Depending on if price1 is higher or lower than price2, the denominator used in the value difference will be different. This leads to asymmetric price valuations.

Lines of code

https://github.com/revert-finance/lend/blob/audit/src/V3Oracle.sol?plain=1#L138

Vulnerability details

When calculating the price difference for two prices, Revert utilizes a formula to calculate the % difference in prices denominated in basis points. Unfortunately, the verifiedPriceX96 used as the denominator is unreliable and leads to incorrect price differences depending on which price (either the price or verified price) is higher.

Impact

Asymmetric price differences will occur depending on which price is higher or lower compared to the other.

Proof of Concept

Below is a forge test which shows how this math formula will return incorrect values based on which price is higher or lower:

function test_priceDiff() public {
    bool r;
    uint verifyPriceX96;
    uint priceX96;
    uint256 maxDifferenceX10000 = 169;

    verifyPriceX96 = 1017;
    priceX96 = 1000;
    uint256 differenceX10000 = priceX96 >= verifyPriceX96 ? (priceX96 - verifyPriceX96) * 10000 : (verifyPriceX96 - priceX96) * 10000;
    r = differenceX10000 / verifyPriceX96 > maxDifferenceX10000;  

    // returns:  result 1:  false 167
    console.log("result 1: ", r, differenceX10000 / verifyPriceX96);

    verifyPriceX96 = 1000;
    priceX96 = 1017;
    differenceX10000 = priceX96 >= verifyPriceX96 ? (priceX96 - verifyPriceX96) * 10000 : (verifyPriceX96 - priceX96) * 10000;
    r = differenceX10000 / verifyPriceX96 > maxDifferenceX10000; 

    // result 2:  true 170
    console.log("result 2: ", r, differenceX10000 / verifyPriceX96);
} 

Tools Used

Manual review

Recommended Mitigation Steps

Instead of relying on the verified price as a denominator in the math formula, utilize the average price between both prices as the denominator.

The following forge test shows how utilizing the average price will result in an accurate price difference regardless of the priceX96 and verifyPriceX96 values.

function test_priceDiffFix() public {
    bool r;
    uint verifyPriceX96;
    uint priceX96;
    uint256 maxDifferenceX10000 = 169;

    verifyPriceX96 = 1017;
    priceX96 = 1000;
    uint256 differenceX10000 = priceX96 >= verifyPriceX96 ? (priceX96 - verifyPriceX96) * 10000 : (verifyPriceX96 - priceX96) * 10000;
    uint256 averagePrice = (priceX96 + verifyPriceX96) / 2;  
    r = differenceX10000 / averagePrice > maxDifferenceX10000;  

    console.log("result 1: ", r, differenceX10000 / averagePrice);

    verifyPriceX96 = 1000;
    priceX96 = 1017;
    differenceX10000 = priceX96 >= verifyPriceX96 ? (priceX96 - verifyPriceX96) * 10000 : (verifyPriceX96 - priceX96) * 10000;
    averagePrice = (priceX96 + verifyPriceX96) / 2;  
    r = differenceX10000 / averagePrice > maxDifferenceX10000; 

    console.log("result 2: ", r, differenceX10000 / averagePrice);
}

Assessed type

Math

Assessed type

Math

[jhsagd76]

Pls note the maxDifferenceX10000 is base points:

uint16 public maxPoolPriceDifference; // max price difference between oracle derived price and pool price x10000

So the only quesion is, whats the denominator of the base points.

Once we determine a unique denominator, these base points will be clearly defined, so I do not see any issues with the current mitigation.

The PoC demonstrated in this issue actually shows that the proportion occupied by 17 will change as the denominator changes.

If we truly choose the average as the denominator, this mathematical formula will evolve into the log2 of base points. For example, with a base of 1 and a numerator of 3, we cannot accept a 200% deviation under any circumstances. However, if using the average as the base, the deviation is only 100%.

[c4-judge]

jhsagd76 marked the issue as nullified

[mrthankyou]

@jhsagd76

Thanks for the explanation. On a closer look you are correct.