Skip to content

fix: security audit fixes for PeripheryImmutableState.sol#43

Open
rroland10 wants to merge 1 commit into
EthereumCommonwealth:mainfrom
rroland10:audit/periphery-immutable-state-security-fixes
Open

fix: security audit fixes for PeripheryImmutableState.sol#43
rroland10 wants to merge 1 commit into
EthereumCommonwealth:mainfrom
rroland10:audit/periphery-immutable-state-security-fixes

Conversation

@rroland10

Copy link
Copy Markdown

Security Audit: PeripheryImmutableState.sol

Summary

Comprehensive security audit of contracts/dex-periphery/base/PeripheryImmutableState.sol — the abstract base contract that stores immutable factory and WETH9 addresses used by all periphery contracts (SwapRouter, NonfungiblePositionManager, Quoter223, V3Migrator, PoolInitializer, LiquidityManagement, PeripheryPayments).


Vulnerabilities Found

1. [MEDIUM] Missing Zero-Address Validation in Constructor

Severity: Medium
Type: Input Validation
Location: PeripheryImmutableState.constructor(address, address)

Description:
The constructor accepts _factory and _WETH9 parameters without validating that they are not address(0). Both variables are declared as immutable, meaning once set at deployment they can never be changed. If either is accidentally set to the zero address during deployment, the entire contract is permanently broken with no recovery path.

Impact:

  • factory = address(0): Every pool lookup via IDex223Factory(factory).getPool(...) and IDex223Factory(factory).createPool(...) would revert or return incorrect results. This breaks SwapRouter (all swap functions), NonfungiblePositionManager (mint, increase/decrease liquidity), PoolInitializer (pool creation), LiquidityManagement (add liquidity), V3Migrator (migration), and Quoter223 (all quote functions). The entire periphery layer becomes non-functional.
  • WETH9 = address(0): The receive() function in PeripheryPayments checks require(msg.sender == WETH9), which would block all native ETH receipts. unwrapWETH9() would call .balanceOf() and .withdraw() on address(0), causing reverts. The pay() function's WETH9 branch would attempt to deposit/transfer on a zero address. V3Migrator's receive() would also be broken, preventing ETH refunds.

Affected child contracts (7):

  • ERC223SwapRouter (SwapRouter.sol)
  • DexaransNonfungiblePositionManager (NonfungiblePositionManager.sol)
  • ERC223Quoter (Quoter223.sol)
  • V3Migrator (V3Migrator.sol)
  • PoolInitializer (PoolInitializer.sol)
  • LiquidityManagement (LiquidityManagement.sol)
  • PeripheryPayments (PeripheryPayments.sol)

Fix: Added require(_factory != address(0), 'FACTORY_ZERO') and require(_WETH9 != address(0), 'WETH9_ZERO') checks at the top of the constructor.


2. [LOW] No Event Emission on Immutable State Initialization

Severity: Low
Type: Observability / Off-Chain Monitoring
Location: PeripheryImmutableState.constructor(address, address)

Description:
The constructor does not emit any event when the immutable state is initialized. Since factory and WETH9 are immutable (not stored in contract storage in the traditional sense), off-chain monitoring tools, indexers, and deployment verification scripts cannot easily discover which addresses were configured at deployment time without parsing constructor arguments from the deployment transaction input data.

Impact:

  • Deployment verification requires manual inspection of transaction calldata rather than simple event log queries
  • Monitoring systems cannot easily track which factory/WETH9 addresses are associated with each periphery deployment
  • Makes multi-chain deployment auditing more difficult

Fix: Added event PeripheryImmutableStateInitialized(address indexed factory, address indexed WETH9) and emit it in the constructor after setting the immutable values.


3. [INFORMATIONAL] Missing NatSpec Documentation on Constructor

Severity: Informational
Type: Code Quality / Documentation
Location: PeripheryImmutableState.constructor(address, address) and contract-level

Description:
The constructor parameters lacked NatSpec @param documentation, and the contract-level documentation only had @title and @notice but was missing @dev documentation explaining the immutability guarantees and implications.

Fix: Added comprehensive NatSpec including @dev at the contract level, @notice, @param, and @dev on the constructor.


Changes Made

 /// @title Immutable state
 /// @notice Immutable state used by periphery contracts
+/// @dev Stores the factory and WETH9 addresses as immutable variables.
+///      These values are set once at construction and can never be changed.
 abstract contract PeripheryImmutableState is IPeripheryImmutableState {
     address public immutable override factory;
     address public immutable override WETH9;

+    event PeripheryImmutableStateInitialized(address indexed factory, address indexed WETH9);
+
+    /// @notice Initializes immutable state for all periphery contracts
+    /// @param _factory The Dex223 factory contract address
+    /// @param _WETH9 The WETH9 contract address
+    /// @dev Reverts if either address is zero to prevent permanently broken deployments
     constructor(address _factory, address _WETH9) {
+        require(_factory != address(0), 'FACTORY_ZERO');
+        require(_WETH9 != address(0), 'WETH9_ZERO');
         factory = _factory;
         WETH9 = _WETH9;
+        emit PeripheryImmutableStateInitialized(_factory, _WETH9);
     }
 }

Compilation Verification

Verified that the changes compile successfully with npx hardhat compile (Solidity 0.7.6, optimizer enabled with 5000 runs). No new errors or warnings introduced. Pre-existing compilation errors in unrelated test files (MockTimeDex223Pool.sol, Dex223PoolLib.sol) remain unchanged.


Test Plan

  • Verify contract compiles with npx hardhat compile (Solidity 0.7.6)
  • Verify deployment with valid factory and WETH9 addresses succeeds
  • Verify deployment with _factory = address(0) reverts with 'FACTORY_ZERO'
  • Verify deployment with _WETH9 = address(0) reverts with 'WETH9_ZERO'
  • Verify PeripheryImmutableStateInitialized event is emitted with correct indexed addresses on successful deployment
  • Verify all child contracts (SwapRouter, NonfungiblePositionManager, Quoter223, V3Migrator) inherit the validation and deploy correctly with valid addresses
  • Verify factory and WETH9 public getters return the correct addresses post-deployment
  • Verify no gas regression on existing deployment flows (constructor-only change, no runtime impact)

Made with Cursor

- Add zero-address validation for _factory and _WETH9 constructor params
- Add PeripheryImmutableStateInitialized event for deployment verification
- Add comprehensive NatSpec documentation for constructor and contract

Co-authored-by: Cursor <cursoragent@cursor.com>
@Dexaran

Dexaran commented Feb 20, 2026

Copy link
Copy Markdown
Member
  1. [MEDIUM] Missing Zero-Address Validation in Constructor

If we will improperly set the variables of our contracts upon deployment then the whole system will not work. It's not something that we will not notice during the launch.

Introducing an extra unnecessary check will only increase the bytecode size without any positive effect.

  1. [LOW] No Event Emission on Immutable State Initialization

Unnecessary increase of the bytecode size.

  1. [INFORMATIONAL] Missing NatSpec Documentation on Constructor

Yea, its reasonable to add the documentation. We can address it a bit later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants