giraffe - _update will overflow and break all contract functionality

[sherlock-admin2]

giraffe

high

_update will overflow and break all contract functionality

Summary

In JalaPair.sol, _update() increments price0CumulativeLast and price1CumulativeLast which are ever-increaasing variables. At some point this will overflow, always revert and break all contract functionality.

Vulnerability Detail

In Jalapair.sol, this is the _update() function:

// update reserves and, on the first call per block, price accumulators
    function _update(uint256 balance0, uint256 balance1, uint112 _reserve0, uint112 _reserve1) private {
        if (balance0 > type(uint112).max || balance1 > type(uint112).max) revert Overflow();
        uint32 blockTimestamp = uint32(block.timestamp % 2 ** 32);
        uint32 timeElapsed = blockTimestamp - blockTimestampLast; // overflow is desired
        if (timeElapsed > 0 && _reserve0 != 0 && _reserve1 != 0) {
            // * never overflows, and + overflow is desired
            price0CumulativeLast += uint256(UQ112x112.encode(_reserve1).uqdiv(_reserve0)) * timeElapsed;
            price1CumulativeLast += uint256(UQ112x112.encode(_reserve0).uqdiv(_reserve1)) * timeElapsed;
        }
        reserve0 = uint112(balance0);
        reserve1 = uint112(balance1);
        blockTimestampLast = blockTimestamp;
        emit Sync(reserve0, reserve1);
    }

This is an exact replica of Uniswap V2 which begs the question why Uniswap doesn't face the same issue.

The reason is because Uniswap V2 uses an older version of Solidity (0.5.16) where overflow does not revert. This is intentional as stated in the code comments that overflow by addition is desired.

However, for Jalaswap this desired overflow cannot happen as it uses a newer version of Solidity (^0.8.0) where overflows are caught and reverted.

This article from Rareskills discusses this bug exactly and states that "Correct modern implementations of the price oracle need to use the unchecked block to ensure everything overflows as expected."

Impact

_update() is called after every contract function (mint, burn swap, sync). So all contract functionality will be broken.

Code Snippet

https://github.com/sherlock-audit/2024-02-jala-swap/blob/main/jalaswap-dex-contract/contracts/JalaPair.sol#L100

Tool used

Manual Review

Recommendation

Wrap line 100 to 101 in an unchecked block to allow for overflow.

Duplicate of #186