Vulnerability Report: Unprotected Reinitializer and Client Registration Squatting

gistfile1.txt

Vulnerability Report: Unprotected Reinitializer and Client Registration Squatting

Executive Summary

Two related access control vulnerabilities exist in the IdentityRegistry and PolicyClientRegistry contracts that enable attackers to disrupt or bypass the identity linking system:

IdentityRegistry.initializeV2() lacks access control, allowing any caller to replace the PolicyClientRegistry address
PolicyClientRegistry.registerClient() allows anyone to register any policy client and become its registry "owner"
These vulnerabilities can result in global denial-of-service for identity linking, bypass of client registration gating, or targeted per-client deactivation attacks.

Vulnerability Details

Issue 1: Unprotected Reinitializer in IdentityRegistry

Location: IdentityRegistry.sol, lines 77-81

Vulnerable Code:

solidity


function initializeV2(
    address _policyClientRegistry
) external reinitializer(2) {
    require(_policyClientRegistry != address(0), "policyClientRegistry required");
    policyClientRegistry = _policyClientRegistry;
}
Problem Analysis:

The initializeV2 function is marked with reinitializer(2) but has no access control modifier (e.g., onlyOwner). This means:

Any external caller can invoke this function exactly once
The caller can set policyClientRegistry to any non-zero address
Once called, the function cannot be called again (reinitializer consumed)
The policyClientRegistry address is critical because _linkIdentity enforces registration checks against it:

solidity


function _linkIdentity(
    address _identityOwner,
    address _policyClient,
    address _clientUser,
    bytes32 _identityDomain
) internal {
    require(
        IPolicyClientRegistry(policyClientRegistry).isRegisteredClient(_policyClient),
        PolicyClientNotRegistered(_policyClient)
    );
    // ...
}
Issue 2: Permissionless Client Registration with Arbitrary Ownership

Location: PolicyClientRegistry.sol, lines 44-57

Vulnerable Code:

solidity


function registerClient(
    address client
) external {
    require(
        IERC165(client).supportsInterface(type(INewtonPolicyClient).interfaceId),
        NotPolicyClient(client)
    );
    require(_clients[client].registeredAt == 0, ClientAlreadyRegistered(client));

    _clients[client] =
        ClientRecord({owner: msg.sender, active: true, registeredAt: uint64(block.timestamp)});
    _ownerClients[msg.sender].add(client);

    emit ClientRegistered(client, msg.sender);
}
Problem Analysis:

The function only validates:

The client address supports INewtonPolicyClient interface (ERC-165 check)
The client is not already registered
It does not validate that msg.sender has any relationship to the client contract. The owner field is set to msg.sender unconditionally.

Once registered, the registry "owner" can:

Call deactivateClient(client) to set active = false
Call setClientOwner(client, newOwner) to transfer ownership
The isRegisteredClient function requires both registration and active status:

solidity


function isRegisteredClient(
    address client
) external view returns (bool) {
    return _clients[client].registeredAt != 0 && _clients[client].active;
}
Exploitation Scenarios

Scenario 1: Global Denial-of-Service via Malicious Registry

Attack Flow:

Attacker deploys a malicious contract implementing IPolicyClientRegistry:
solidity


contract MaliciousRegistry {
    function isRegisteredClient(address) external pure returns (bool) {
        return false; // Or: revert("blocked");
    }
}
Attacker calls IdentityRegistry.initializeV2(maliciousRegistryAddress)
All subsequent calls to any linkIdentity* function fail at:
solidity


require(
    IPolicyClientRegistry(policyClientRegistry).isRegisteredClient(_policyClient),
    PolicyClientNotRegistered(_policyClient)
);
Preconditions:

IdentityRegistry proxy has been deployed and initialize() was called
initializeV2() has not yet been called (reinitializer slot 2 is available)
Impact: Complete halt of all new identity linking operations. Existing links remain intact but no new links can be created.

Scenario 2: Global Gating Bypass via Permissive Registry

Attack Flow:

Attacker deploys a malicious contract:
solidity


contract PermissiveRegistry {
    function isRegisteredClient(address) external pure returns (bool) {
        return true; // Always passes
    }
}
Attacker calls IdentityRegistry.initializeV2(permissiveRegistryAddress)
Any policy client address (even unregistered or malicious ones) passes the registration check
Preconditions:

Same as Scenario 1
Impact: The client registration gating mechanism is completely nullified. Note that signature verification still applies, so attackers cannot forge identity links without valid signatures from both parties.

Scenario 3: Per-Client Denial-of-Service via Registration Squatting

Attack Flow:

Legitimate developer deploys a policy client contract V that implements INewtonPolicyClient
Before the developer registers V, attacker calls:
solidity


PolicyClientRegistry.registerClient(V)
Attacker becomes the registry "owner" of V
Attacker calls:
solidity


PolicyClientRegistry.deactivateClient(V)
Any attempt to link identities to client V fails because:
solidity


isRegisteredClient(V) // returns false (active = false)
The legitimate developer cannot:
Register V again (already registered)
Activate V (not the registry owner)
Reclaim ownership (no admin mechanism exists)
Preconditions:

A policy client contract exists that supports INewtonPolicyClient interface
The contract has not yet been registered in PolicyClientRegistry
Attacker can submit a transaction before the legitimate owner
Impact: Permanent denial-of-service for the targeted policy client. The only recovery would be to deploy a new policy client contract.

Impact Assessment

Factor	Assessment
Confidentiality	No direct impact on stored identity data
Integrity	Gating bypass allows unintended clients to pass checks
Availability	Complete DoS of identity linking possible
Scope	Global (all clients) or targeted (specific clients)
Reversibility	reinitializer consumed; squatted registrations are permanent
Severity: HIGH

The attacks require no special privileges
Global DoS is achievable with a single transaction
No on-chain recovery mechanism exists for either attack vector
Remediation Approach

Fix 1: Add Access Control to initializeV2

Fixed Code:

solidity


function initializeV2(
    address _policyClientRegistry
) external reinitializer(2) onlyOwner {
    require(_policyClientRegistry != address(0), "policyClientRegistry required");
    policyClientRegistry = _policyClientRegistry;
}
Adding onlyOwner ensures only the contract owner can set the registry address during upgrade.

Fix 2: Validate Client Ownership During Registration

Fixed Code:

solidity


function registerClient(
    address client
) external {
    require(
        IERC165(client).supportsInterface(type(INewtonPolicyClient).interfaceId),
        NotPolicyClient(client)
    );
    require(_clients[client].registeredAt == 0, ClientAlreadyRegistered(client));

    address owner = INewtonPolicyClient(client).getOwner();
    require(owner != address(0), InvalidOwnerAddress());
    require(msg.sender == owner || msg.sender == client, NotClientOwner(client, msg.sender));

    _clients[client] =
        ClientRecord({owner: owner, active: true, registeredAt: uint64(block.timestamp)});
    _ownerClients[owner].add(client);

    emit ClientRegistered(client, owner);
}
This fix:

Queries the client contract for its actual owner via getOwner()
Requires msg.sender to be either the owner or the client contract itself
Sets the registry owner to the actual client owner (not msg.sender)
Prerequisite: INewtonPolicyClient must expose a getOwner() function that returns the legitimate owner address.

Open Questions for Review

Interface Requirement: Does INewtonPolicyClient currently define a getOwner() function? If not, it must be added and all implementing contracts must be updated.
Upgrade Timing: For existing deployments where initializeV2() may have already been called legitimately, verify the reinitializer slot state before deploying fixes.
Squatted Entries: If any policy clients have already been squatted in production, how should they be remediated? Consider adding an admin function to correct ownership or re-register clients.
Registry Admin Functions: Should PolicyClientRegistry include admin override capabilities (e.g., forceSetClientOwner) for emergency recovery? This introduces centralization tradeoffs.
Proxy Upgrade Path: If the registry address was already maliciously set via initializeV2, the fix must include a mechanism to correct it (potentially requiring a new reinitializer version or admin setter function).
Verification Steps

After applying fixes, verify:

initializeV2 Access Control:
Non-owner calls to initializeV2() revert
Owner can successfully call initializeV2() (if not already consumed)
Client Registration Ownership:
Registration fails if msg.sender is not the client's actual owner
Registration succeeds when called by the legitimate owner
Registration succeeds when called by the client contract itself
Registry owner matches the client's getOwner() return value
Existing Functionality:
Legitimate identity linking still works after fixes
Client activation/deactivation works for legitimate owners
Ownership transfer works correctly