fix: Security Audit – PoolTicksCounter.sol (6 vulnerabilities)

[rroland10]

Security Audit Report: PoolTicksCounter.sol

Executive Summary

A thorough security audit of contracts/dex-periphery/base/PoolTicksCounter.sol identified 6 vulnerabilities (1 High, 2 Medium, 2 Low, 1 Informational/Gas). The contract is a direct port of Uniswap V3's PoolTicksCounter library and inherits all upstream issues, compounded by the use of Solidity >=0.6.0 (compiled with 0.7.6 in this project), which lacks built-in overflow/underflow protection.

This PR fixes all identified issues while maintaining full backward compatibility with the existing interface and behavior.

---

Vulnerability Details

V-01 — uint32 Underflow on initializedTicksCrossed Subtraction (HIGH)

Location: Lines 77-83 (original)

Description: The function subtracts 1 from initializedTicksCrossed (a uint32) when tickAfterInitialized or tickBeforeInitialized is true. In Solidity <0.8, unsigned integer subtraction silently wraps on underflow. If initializedTicksCrossed is 0 when either subtraction executes, the result becomes type(uint32).max (4,294,967,295), returning a wildly incorrect tick count.

Attack scenario: This can occur when the bitmap iteration finds no initialized ticks between the boundary ticks, but the boundary tick detection flags still evaluate to true due to specific tick alignments. Any consumer relying on this count for gas estimation (e.g., QuoterV2) would receive drastically wrong values, potentially causing transaction failures or incorrect fee calculations.

Fix: Added initializedTicksCrossed > 0 guard before each subtraction.

---

V-02 — Negative Tick Modulo Produces Incorrect bitPos (MEDIUM)

Location: Lines 25-26 (original)

Description: The original code computes bitPos as uint8((tickBefore / self.tickSpacing()) % 256). In Solidity <0.8, the modulo operator preserves the sign of the dividend: (-5) % 256 == -5, not 251. When a negative compressed tick is not evenly divisible by 256, the modulo returns a negative value. Casting this negative int24 to uint8 produces an incorrect bit position through two's complement reinterpretation.

Additionally, Solidity signed integer division truncates towards zero (not towards negative infinity), so (-1) / 10 == 0 instead of the expected -1. This means the compressed tick index itself is wrong for negative non-aligned ticks.

Impact: Incorrect bitPos values cause the function to look up wrong bits in the tick bitmap, leading to incorrect initialized-tick counts for negative tick ranges. This affects gas estimation accuracy for swaps involving negative price ranges.

Fix: Introduced _compress() for floor-division and _position() for safe bit decomposition using bitwise AND on two's complement representation.

---

V-03 — int16 Truncation on Extreme wordPos Values (MEDIUM)

Location: Lines 25, 28 (original)

Description: The original code casts the shifted compressed tick directly to int16: int16((tickBefore / self.tickSpacing()) >> 8). For extreme tick values near MIN_TICK (-887272) with small tick spacings (e.g., 1), the compressed tick divided by 256 can exceed the int16 range of [-32768, 32767]. The int16() cast in Solidity <0.8 silently truncates, causing the function to look up the wrong bitmap word entirely.

Impact: For pools with tick spacing of 1 and swaps near extreme tick boundaries, the function returns incorrect counts due to bitmap word aliasing.

Fix: The _position() helper uses arithmetic right-shift (>> 8) which correctly floors for signed integers, and the _compress() function ensures the compressed tick is properly floor-divided before splitting, keeping values within the valid range for the bitmap.

---

V-04 — int16 Overflow in wordPosLower++ Loop (LOW)

Location: Line 72 (original)

Description: The while loop increments wordPosLower (an int16) unconditionally after processing each bitmap word. If wordPosLower equals type(int16).max (32767), the increment silently wraps to -32768 in Solidity <0.8, making the condition wordPosLower <= wordPosHigher perpetually true and causing an infinite loop that consumes all remaining gas.

Impact: While unlikely in practice (would require iterating through ~65,000 bitmap words), the theoretical DoS vector exists for extreme tick ranges.

Fix: Added an explicit break when wordPosLower == wordPosHigher before the increment, preventing the overflow scenario.

---

V-05 — countOneBits Documentation (LOW / INFORMATIONAL)

Location: Line 88 (original)

Description: The countOneBits function uses the Kernighan bit-counting algorithm but lacks documentation. The algorithm is O(k) where k is the number of set bits, which can be up to 256 iterations for a full bitmap word. A constant-time Hamming weight / parallel bit count implementation would be more gas-efficient.

Fix: Added NatSpec documentation. The Kernighan algorithm is retained to minimize diff churn and maintain readability, as gas savings from a constant-time popcount are marginal in the typical case (sparse bitmaps).

---

V-06 — Redundant External Calls to tickSpacing() (GAS / INFORMATIONAL)

Location: Lines 25-26, 28-29, 37, 44 (original)

Description: self.tickSpacing() is called up to 6 times, each invocation being an external STATICCALL costing ~700 gas (cold) or ~100 gas (warm). The return value is immutable for a given pool, so it should be cached.

Fix: Cache tickSpacing() in a local variable at the start of the function. Estimated savings: ~500-4,000 gas per call depending on cold/warm access patterns.

---

Files Changed

File	Changes
contracts/dex-periphery/base/PoolTicksCounter.sol	+68 / -21

Compilation

Verified: The modified contract compiles successfully under the project's Hardhat configuration (Solidity 0.7.6 with optimizer enabled). Pre-existing compilation errors in unrelated files (Dex223PoolLib.sol, MockTimeDex223Pool.sol) remain unchanged.

---

Test Plan

• Unit: Underflow guard (V-01) — Call countInitializedTicksCrossed() with tickBefore and tickAfter straddling an empty bitmap region where boundary ticks are initialized. Verify result is 0, not type(uint32).max.
• Unit: Negative tick modulo (V-02) — Test with negative ticks that are not multiples of tickSpacing (e.g., tickBefore = -5, tickSpacing = 10). Verify bitPos maps to the correct bitmap bit.
• Unit: Extreme tick values (V-03) — Test with ticks near MIN_TICK (-887272) and MAX_TICK (887272) with tickSpacing = 1. Verify no truncation or incorrect word lookups.
• Unit: Large tick range iteration (V-04) — Test with a tick range spanning many bitmap words to verify the loop terminates correctly without overflow.
• Unit: Upward swap counting — Verify that tickBefore is excluded and tickAfter is included when tickAfter > tickBefore.
• Unit: Downward swap counting — Verify that tickAfter is excluded and tickBefore is included when tickBefore > tickAfter.
• Unit: Same tick (no swap) — Verify countInitializedTicksCrossed() returns 0 when tickBefore == tickAfter.
• Integration: Gas comparison — Compare gas usage before and after the tickSpacing() caching optimization (V-06).
• Regression: Existing test suite — Run the full PoolTicksCounterTest contract test suite and verify all existing tests pass.
• Compilation: Clean build — Verify npx hardhat compile produces no new errors or warnings related to PoolTicksCounter.sol.

[Dexaran]

V-01 - V-05

These are all math-related reports. We know how Uniswap math works and even though its imperfect - its not really worth changing at this point.

V-06 — Redundant External Calls to tickSpacing()

That might be useful in theory. Lets look into it when we will be working on features for the next release.