mahmud - DOS in JalaPair contract in critical functions like `mint`, `burn` and `swap` due to overflow.

[sherlock-admin]

mahmud

high

DOS in JalaPair contract in critical functions like mint, burn and swap due to overflow.

Summary

The timeElapsed variable and the price0CumulativeLast/price1CumulativeLast state variables in the _update function are expected to overflow for the function to work properly. However due to solidity compiler's (version 0.8.0 and above) check for overflow, the function automatically reverts causing a DOS when minting, burning and swapping.

Vulnerability Detail

In the JalaPair contract, the _update function updates reserves and performs some other logic to handle the TWAP oracle including calculating the timeElapsed and the priceCumulatives. The function is called internally during critical operations like adding liquidity(minting), burning liquidity and swapping. Both the timeElapsed variable and the price0CumulativeLast/price1CumulativeLast state variables are expected to overflow for the function to work properly. However, the function reverts during overflow due to the solidity compiler's check for underflow and overflow. If the current block.timestamp is greater than type(uint32).max and the last time the blockTimestampLast was updated was when block.timestamp was less than type(uint32).max an overflow occurs, also if price0CumulativeLast/price1CumulativeLast gets close to type(uint256).max (which is possible and can happen fast if the pair is traded often), overflow also occurs and the transaction reverts preventing swaps, minting and burning.

Impact

DOS in essential transactions like swapping, minting and burning. Preventing liquidity providers from adding liquidity, burning liquidity and swapping for users.

Code Snippet

https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/JalaPair.sol#L94-L107

Tool used

Manual Review

Recommendation

Place the timeElapsed variable and the price0CumulativeLast/price1CumulativeLast state variables in the _update function in an unchecked block to allow overflow as expected

function _update(
    uint256 balance0,
    uint256 balance1,
    uint112 reserve0_,
    uint112 reserve1_
) private {
    ...
+    unchecked {
+        uint32 timeElapsed = blockTimestamp - blockTimestampLast;
+
+        if (timeElapsed > 0 && _reserve0 != 0 && _reserve1 != 0) {
+            price0CumulativeLast +=
+                uint256(UQ112x112.encode(reserve1_).uqdiv(reserve0_)) *
+                timeElapsed;
+            price1CumulativeLast +=
+                uint256(UQ112x112.encode(reserve0_).uqdiv(reserve1_)) *
+                timeElapsed;
+        }
+    }

    reserve0 = uint112(balance0);
    reserve1 = uint112(balance1);
    blockTimestampLast = blockTimestamp;

    ...
}

Duplicate of #186