Arithmetic Overflow is caused while calculating Liquidity Quote during a Hot Swap

Summary

While calculation of Liquidity Quote during a Hot Swap , some token pairs cause Arithmetic Overflow error. This bug prevents swapping of certain token pairs beyond limit of tokens, creating an uneven situation for the protocol .

Vulnerability Detail

HOT#getLiquidityQuote function is called by the Sovereign Pool to handle the underlying pricing logic for swaps. It is divided into two parts on whether the external context is provided or not . If provided , then it indicates a hot swap is to be done & the calculations are done using HOT#_hotSwap . If not, an AMM swap should be performed & calculations are done using HOT#_ammSwap . The same can be seen in code provided below :

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOT.sol#L631C4-L650C6

function getLiquidityQuote(
        ALMLiquidityQuoteInput memory _almLiquidityQuoteInput,
        bytes calldata _externalContext,
        bytes calldata /*_verifierData*/
    ) external override onlyPool onlyUnpaused returns (ALMLiquidityQuote memory liquidityQuote) {
        if (_externalContext.length == 0) {
            // AMM Swap
            _ammSwap(_almLiquidityQuoteInput, liquidityQuote);
        } else {
            // Hot Swap
            _hotSwap(_almLiquidityQuoteInput, _externalContext, liquidityQuote);

            // Hot swap needs a swap callback, to update AMM liquidity correctly
            liquidityQuote.isCallbackOnSwap = true;
        }

        if (liquidityQuote.amountOut == 0) {
            revert HOT__getLiquidityQuote_zeroAmountOut();
        }
    }

Now when HOT#_hotSwap function is called by HOT#getLiquidityQuote function , the underlying calculations done in HOT#_hotSwap to calculate the liquidityQuote.amountOut are as shown below : https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOT.sol#L921C9-L930C34

liquidityQuote.amountOut = almLiquidityQuoteInput.isZeroToOne
            ? (
                Math.mulDiv(
                    almLiquidityQuoteInput.amountInMinusFee * sqrtHotPriceX96,
                    sqrtHotPriceX96,
                    HOTConstants.Q192
                )
            )
            : (Math.mulDiv(almLiquidityQuoteInput.amountInMinusFee, HOTConstants.Q192, sqrtHotPriceX96) /                      <@audit 
                sqrtHotPriceX96);

Here when the flag isZeroToOne is set to false, will cause an Overflow for certain tokens . To understand this Let's take an example , let's assume the pair used by User for swap is USDC/DAI , According to documentation , the entire ALMLiquidityQuoteInput struct contains the user's requested swap along with the fees deducted . Let's say the User is hoping to swap 2100 DAI tokens for USDC . After deducting the fees , let's say the almLiquidityQuoteInput.amountInMinusFee is about ~2000 DAI tokens & the constant HOTConstants.Q192 is approximately ~10^57 in the power of 10 . So the numerator for the first section of calculation will be :

                    numerator1 = almLiquidityQuoteInput.amountInMinusFee x HOTConstants.Q192
                               = 2000 x 10^18 x 10^57
                               ~ 2 x 10^78

But we know that uint256.max is about ~10^77 . This causes an unexpected Overflow error , stopping the swap from completion .

Impact

DOS of the protocol during swapping of certain token pairs caused by Arithmetic Overflow

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOT.sol#L631C4-L650C6

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOT.sol#L921C9-L930C34

Tool used

Manual Review

Recommendation

Instead of Multiplying the amountInMinusFee with entire Q192 constant , divide it and use half of it in first part of calculation and the other half in the second part as shown below :

Math.mulDiv(Math.mulDiv(almLiquidityQuoteInput.amountInMinusFee, HOTConstants.Q96, sqrtHotPriceX96), HOTConstants.Q96 , sqrtHotPriceX96) ;

The same can be done for the case when isZeroToOne is set to true , to prevent Arithmetic Errors .

sherlock-audit / 2024-03-arrakis-judging

Angry_Mustache_Man - Arithmetic Overflow is caused while calculating Liquidity Quote during a Hot Swap #52