fix: Security Audit Fixes for DataHelper.sol

[rroland10]

Security Audit Report: DataHelper.sol

Contract Overview

DataHelper.sol is a pure utility contract (no state, no ETH handling) that constructs MintParams structs used by the Dex223 position manager to mint liquidity positions. The contract previously had zero input validation, meaning any caller — including other contracts composing on top of it — could silently construct invalid parameters that would fail downstream in the position manager, wasting gas and potentially causing confusing reverts.

---

Vulnerabilities Found & Fixes Applied

V1 — MEDIUM: Zero-Address Token Parameters Accepted

Severity	Medium
Category	Input Validation
Status	Fixed

Description: The original MintParamsCall function accepted address(0) for token0 and/or token1 without any check. Passing a zero-address token to the Uniswap V3 position manager would cause a downstream revert, but only after additional external calls have already been made, wasting gas. Worse, if this helper is used to build calldata off-chain or in another contract, the invalid struct propagates silently.

Fix: Added require(token0 != address(0)) and require(token1 != address(0)) checks at the top of the function.

---

V2 — MEDIUM: Identical Token Addresses Accepted

Severity	Medium
Category	Input Validation
Status	Fixed

Description: The function would happily construct a MintParams struct where token0 == token1. This is invalid for any Uniswap V3 pool and would revert downstream.

Fix: Added require(token0 != token1, "DataHelper: identical tokens").

---

V3 — MEDIUM: No Token Ordering Enforcement

Severity	Medium
Category	Input Validation / Invariant Enforcement
Status	Fixed

Description: Uniswap V3 strictly requires token0 < token1 (sorted by address). The original function did not enforce this, so callers could construct params with reversed ordering that would fail at the pool level. This is especially dangerous for integrators who may not be aware of the ordering requirement.

Fix: Added require(token0 < token1, "DataHelper: token0 must be less than token1").

---

V4 — MEDIUM: Invalid Tick Ranges Accepted

Severity	Medium
Category	Input Validation
Status	Fixed

Description: The function accepted tickLower >= tickUpper, which is invalid for Uniswap V3 positions. An inverted or zero-width tick range would revert in the pool contract but only after significant computation.

Fix: Added require(tickLower < tickUpper, "DataHelper: tickLower must be less than tickUpper").

---

V5 — LOW: Minimum Amounts Could Exceed Desired Amounts

Severity	Low
Category	Input Validation / Logic Error Prevention
Status	Fixed

Description: amount0Min > amount0Desired or amount1Min > amount1Desired is a logical impossibility — the minimum accepted amount cannot exceed the desired amount. This would guarantee the downstream mint always reverts. The original code allowed this silently.

Fix: Added require(amount0Min <= amount0Desired) and require(amount1Min <= amount1Desired).

---

V6 — LOW: Zero-Address Recipient Accepted (LP Token Burning)

Severity	Low
Category	Input Validation / Fund Safety
Status	Fixed

Description: A recipient of address(0) would cause minted LP position NFTs to be sent to the zero address, effectively burning them permanently. While the Uniswap V3 position manager may or may not reject this, the helper should prevent it at the earliest possible point.

Fix: Added require(recipient != address(0), "DataHelper: recipient is zero address").

---

V7 — LOW: Zero Deadline Accepted (Immediate Expiry)

Severity	Low
Category	Input Validation
Status	Fixed

Description: A deadline of 0 means the transaction's deadline has already passed at block.timestamp > 0, causing an immediate revert downstream. This wastes gas and provides no useful error message to the caller.

Fix: Added require(deadline > 0, "DataHelper: deadline is zero").

---

V8 — INFORMATIONAL: Function Naming Convention Violation

Severity	Informational
Category	Code Quality / Standards
Status	Fixed

Description: The function MintParamsCall uses PascalCase, which in Solidity convention is reserved for contract names, struct names, and event names. Functions should use camelCase per the Solidity Style Guide.

Fix: Renamed to mintParamsCall. Verified no other files in the repository reference the old name.

---

V9 — INFORMATIONAL: Missing NatSpec Documentation

Severity	Informational
Category	Code Quality / Developer Experience
Status	Fixed

Description: The contract had zero NatSpec comments. For a utility contract intended to be called by external integrators, documentation is critical for safe usage.

Fix: Added full NatSpec @title, @notice, @dev, @param, and @return tags for both the contract and the function.

---

Compilation Status

• DataHelper.sol compiles cleanly with Solidity 0.7.6 and the project's Hardhat configuration.
• Pre-existing compilation errors in unrelated files (Dex223PoolLib.sol, MockTimeDex223Pool.sol) are not affected by these changes.
• No other files in the repository reference MintParamsCall, so the rename is a non-breaking change.

---

Summary Table

ID	Severity	Description	Status
V1	Medium	Zero-address token0/token1 accepted	Fixed
V2	Medium	Identical token addresses accepted	Fixed
V3	Medium	No Uniswap V3 token ordering enforcement	Fixed
V4	Medium	Invalid tick ranges (tickLower >= tickUpper) accepted	Fixed
V5	Low	Min amounts could exceed desired amounts	Fixed
V6	Low	Zero-address recipient accepted (LP burning risk)	Fixed
V7	Low	Zero deadline accepted (immediate expiry)	Fixed
V8	Informational	PascalCase function name violates convention	Fixed
V9	Informational	Missing NatSpec documentation	Fixed

---

Test Plan

• Compilation: Verify DataHelper.sol compiles with Solidity 0.7.6 (npx hardhat compile)
• V1 – Zero-address tokens: Call mintParamsCall with token0 = address(0) → should revert with "DataHelper: token0 is zero address"
• V1 – Zero-address tokens: Call mintParamsCall with token1 = address(0) → should revert with "DataHelper: token1 is zero address"
• V2 – Identical tokens: Call with token0 == token1 (both non-zero) → should revert with "DataHelper: identical tokens"
• V3 – Token ordering: Call with token0 > token1 → should revert with "DataHelper: token0 must be less than token1"
• V4 – Tick range: Call with tickLower == tickUpper → should revert with "DataHelper: tickLower must be less than tickUpper"
• V4 – Tick range: Call with tickLower > tickUpper → should revert with "DataHelper: tickLower must be less than tickUpper"
• V5 – Min > Desired: Call with amount0Min > amount0Desired → should revert with "DataHelper: amount0Min exceeds amount0Desired"
• V5 – Min > Desired: Call with amount1Min > amount1Desired → should revert with "DataHelper: amount1Min exceeds amount1Desired"
• V6 – Zero recipient: Call with recipient = address(0) → should revert with "DataHelper: recipient is zero address"
• V7 – Zero deadline: Call with deadline = 0 → should revert with "DataHelper: deadline is zero"
• Happy path: Call with all valid parameters → should return correctly populated MintParams struct
• Boundary: min == desired: Call with amount0Min == amount0Desired → should succeed
• Boundary: adjacent ticks: Call with tickUpper = tickLower + 1 → should succeed
• Integration: Verify no other contracts in the repository break due to the function rename
• Gas check: Confirm added validation does not significantly impact gas for valid calls

Made with Cursor