Initialization Deadlock in KatanaV3Pool Due to Conflicting Constructor and initializeImmutables Logic

[howlbot-integration]

Lines of code

https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/core/KatanaV3Pool.sol#L111
https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/core/KatanaV3Pool.sol#L117

Vulnerability details

Summary

The KatanaV3Pool contract has an initialization deadlock where the constructor sets factory = address(1) while initializeImmutables() requires factory == address(0) to proceed. This creates a paradox. The initialization protection mechanism conflicts with the proxy pattern design, creating a deadlock where the very check meant to protect initialization makes it impossible.

constructor() {
    factory = address(1);  // Sets factory to 1
}

function initializeImmutables(...) public virtual override {
    require(factory == address(0), "AII");  // Requires factory to be 0
    // ... initialization logic that can never be reached
}

Impact

• Pool initialization is permanently blocked
• Proxy deployment pattern becomes unusable
• Contract parameters cannot be set
• Pool cannot be used for its intended purpose

The problem stems from a contradiction in the initialization logic. The constructor immediately sets factory = address(1), but the initializeImmutables function requires factory == address(0) to proceed. This creates a logical impossibility - the initialization function can never be called successfully because its primary guard condition can never be satisfied.

Scenario

Let's walk through the execution flow:

1. When the contract is deployed, the constructor executes:

constructor() {
    factory = address(1);
}

This permanently sets the factory address to 1. The comment suggests this is intended to "disable immutables initialization", but this creates issues with the proxy pattern implementation.

2. Later, when someone attempts to initialize the pool through initializeImmutables, the first check is:

require(factory == address(0), "AII");

This check will always fail because factory is already set to address(1). The error "AII" (Already Initialized Immutables) will be thrown in every case.

This becomes particularly problematic when considering the contract's intended usage with proxy patterns, as indicated by the comment:

// These below immutable constants are set when deploying the beacon proxy of the pool and
// cannot be changed unless upgraded.

Fix

Remove the constructor initialization of factory since it's breaking the proxy initialization pattern:

// BEFORE - Problematic constructor
constructor() {
    // disable immutables initialization
    factory = address(1);  // This prevents initialization
}

// AFTER - Fixed constructor
constructor() {
    // Remove factory initialization entirely
    // Let proxy handle initialization properly
}

The initialization check in initializeImmutables() can stay as is:

function initializeImmutables(...) public virtual override {
    require(factory == address(0), "AII");  // Now this can actually pass during proxy initialization
    // ... rest of the initialization logic
}

The fixed version is better because:

1. The proxy pattern already provides initialization safety
2. The factory=1 setting was an unnecessary and problematic protection
3. The require(factory == address(0)) check is sufficient for ensuring single initialization

This allows the proxy deployment pattern to work as intended while maintaining the one-time initialization guarantee through the zero-address check.

Assessed type

Context

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid