sherlock-audit / 2024-02-jala-swap-judging

6 stars 4 forks source link

santiellena - `JalaPair::_update` reverts due to underflow #196

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 8 months ago

santiellena

high

JalaPair::_update reverts due to underflow

Summary

The JalaPair::_update function reverts due to the desired overflow inherited from UniswapV2 fork logic.

Vulnerability Detail

When calculating timeElapsed, the following calculation is made: uint32 timeElapsed = blockTimestamp - blockTimestampLast;. Where uint32 blockTimestamp = uint32(block.timestamp % 2 ** 32); and blockTimestampLast is the result of the same calculation but on the previous call to the _update function. This underflow situation is really likely to happen, in fact the code has a comment saying that overflow is desired.

Because of this, some transactions will go through and others won't depending on the block.timestamp value.

Impact

The whole functionality of the DEX will be affected and working intermittently. Users will experience funds unavailability.

Code Snippet

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

Tool used

Manual Review

Recommendation

To let the calculation overflow or underflow, add the unchecked label to the code:

    // 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
+       uint32 timeElapsed;
+       unchecked {
+          timeElapsed = blockTimestamp - blockTimestampLast;
+       }
        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);
    }

Duplicate of #186