code-423n4 / 2024-07-basin-validation

0 stars 0 forks source link

Inaccurate Reserve Initialization in `calcReserveAtRatioLiquidity` Function Leads to Suboptimal Swap Pricing #114

Open c4-bot-2 opened 3 months ago

c4-bot-2 commented 3 months ago

Lines of code

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

Vulnerability details

Vulnerability Details

The calcReserveAtRatioLiquidity function tries to update scaledReserves[j] based on the pd.lutData (the data from the lookup table) and the scaledReserves[i] value. However, this approach is not ideal. If the target price (pd.targetPrice) is closer to the pd.lutData.highPrice, the function sets scaledReserves[j] based on the pd.lutData.highPriceJ value. Similarly, if the target price is closer to the pd.lutData.lowPrice, the function sets scaledReserves[j] based on the pd.lutData.lowPriceJ value.

Code

 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;

The problem with this logic is that these price ratios (pd.lutData.highPriceJ and pd.lutData.lowPriceJ) may not necessarily be accurate enough to correctly set the initial value of scaledReserves[j]. This can lead to the loop not converging to the desired result within the 255 iterations.

Impact

The calcReserveAtRatioLiquidity function is used to calculate the reserves for swap operations. If the reserve values are not calculated accurately, it can lead to suboptimal swap pricing and potentially less efficient swap executions.

In the worst-case scenario, the inaccurate reserve calculations could lead to capital loss for users providing liquidity or executing swaps, as the pool may not be able to maintain the expected parity between the token reserves.

Poc

function test_calcReserveAtRatioLiquidity_initializeReserve() public {
    uint256[] memory reserves = new uint256[](2);
    reserves[0] = 1e18; 
    reserves[1] = 1e6; 
    uint256[] memory ratios = new uint256[](2);
    ratios[0] = 1e6; 
    ratios[1] = 1e6;
    _data = abi.encode(18, 6);

    uint256 reserve = _function.calcReserveAtRatioLiquidity(reserves, 1, ratios, _data);
    uint256 expectedReserve = reserves[0] * ratios[1] / ratios[0];

    assertApproxEqRel(reserve, expectedReserve, 1e-6);
}

In this test, I set up a scenario with different decimal places for the two tokens (18 and 6 decimals). We then call the calcReserveAtRatioLiquidity function and check that the returned reserve value is close to the expected value, which is calculated directly from the provided ratios and the other reserve.

Mitigation

Consider using a more robust method to initialize the scaledReserves[j] value, such as calculating it directly from the scaledReserves[i] and the target price ratio, rather than relying on the lookup table data.

Assessed type

Math

nevillehuang commented 3 months ago

Could be related to #44