Incorrect reserve updates for when `targetPrice` is exactly in the middle of high and low price

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-07-basin/blob/7d5aacbb144d0ba0bc358dfde6e0cc913d25310e/src/functions/Stable2.sol#L198-L211 https://github.com/code-423n4/2024-07-basin/blob/7d5aacbb144d0ba0bc358dfde6e0cc913d25310e/src/functions/Stable2.sol#L265-L279

Vulnerability details

Impact

The returned reserve will be incorrectly calculated in the calcReserveAtRatioSwap and calcReserveAtRatioLiquidity functions due to pd.currentPrice incorrectly set as high price when targetPrice lies directly in the middle of highPrice and Lowprice.

Proof of Concept

1. Context

We look first at calcReserveAtRatioLiquidity function. The pd.CurrentPrice parameter is updated depending on if the pd.targetPrice is closer to pd.lutData.highPrice or pd.lutData.lowPrice . This is enforced by comparing the differences between them and the target price before settling pd.currentPrice to low/high price. This is also corroborated by the comments.

        // update scaledReserve[j] such that calcRate(scaledReserves, i, j) = low/high Price,
        // depending on which is closer to targetPrice.
        if (pd.lutData.highPrice - pd.targetPrice > pd.targetPrice - pd.lutData.lowPrice) {
            // targetPrice is closer to lowPrice.
            scaledReserves[j] = scaledReserves[i] * pd.lutData.lowPriceJ / pd.lutData.precision;

            // set current price to lowPrice.
            pd.currentPrice = pd.lutData.lowPrice;
        } else {
            // targetPrice is closer to highPrice.
            scaledReserves[j] = scaledReserves[i] * pd.lutData.highPriceJ / pd.lutData.precision;

            // set current price to highPrice.
            pd.currentPrice = pd.lutData.highPrice;
        }

This can also be observed in the calcReserveAtRatioSwap function.

        // update `scaledReserves` based on whether targetPrice is closer to low or high price:
        if (pd.lutData.highPrice - pd.targetPrice > pd.targetPrice - pd.lutData.lowPrice) {
            // targetPrice is closer to lowPrice.
            scaledReserves[i] = parityReserve * pd.lutData.lowPriceI / pd.lutData.precision;
            scaledReserves[j] = parityReserve * pd.lutData.lowPriceJ / pd.lutData.precision;
            // initialize currentPrice:
            pd.currentPrice = pd.lutData.lowPrice;
        } else {
            // targetPrice is closer to highPrice.
            scaledReserves[i] = parityReserve * pd.lutData.highPriceI / pd.lutData.precision;
            scaledReserves[j] = parityReserve * pd.lutData.highPriceJ / pd.lutData.precision;
            // initialize currentPrice:
            pd.currentPrice = pd.lutData.highPrice;
        }

2. Bug Location

The functions however ignore an edgecase in which the value of pd.lutData.highPrice - pd.targetPrice is exactly equal to pd.targetPrice - pd.lutData.lowPrice, meaning there's no price deviations, and as such current price should remain the same as its target. This is because the function directly uses the else block, which handles the <= aspects of the operation.

3. Final Effect

This means that when pd.lutData.highPrice - pd.targetPrice == pd.targetPrice - pd.lutData.lowPrice, the price difference on both sides, hence, not closer to any, the currentPrice which should be set to the targetPrice, i.e the desired price is wrongly shifted higher. This incorrect price is then used to calculate the scaledReserves potentially returning incorrect values for the reserves in both function.

Tools Used

Manual code review

Recommended Mitigation Steps

Recommend handling this edgecase and setting pd.currentPrice to targetPrice instead, when there are no deviations in its proximity to the high or low price.

Assessed type

Context