Opus finding
CloneERC20Factory initializes the implementation contract directly (no _disableInitializers), exposing it to ownership takeover
mediumsecurityhigh
- src/tokens/CloneERC20Factory.sol:19-24
- src/tokens/CloneERC20.sol:109-130
`CloneERC20` (unlike `DopplerERC20V1`, which calls `_disableInitializers()` in its constructor) relies on the factory invoking `initialize` once at deploy time to lock the initializer on the implementation contract. The implementation is then no longer re-initializable. However, `_initializeOwner(address(0))` and the resulting `owner == address(0)` state means subsequent calls like `lockPool`/`unlockPool`/`updateMintRate`/`updateTokenURI` on the implementation revert under solady Ownable's `onlyOwner` (since msg.sender is never address(0)). But solady Ownable has no two-step ownership transfer guard; if `_initializeOwner(address(0))` makes the implementation owner-less, that's fine. The bigger concern is whether the initialize-lock holds: solady Initializable's `initializer` modifier should mark it done. Provided that's true, this is safe. The deviation from DopplerERC20V1's pattern (which uses `_disableInitializers()`) is a maintainability concern — the two clones diverge in their initialization-locking convention.
Recommendation
Align by adding `constructor() { _disableInitializers(); }` to `CloneERC20` rather than calling `initialize` with zeroed args in the factory. This avoids polluting storage and matches DopplerERC20V1.