ONCHAINID Audit

Table of Contents

Scope

OpenZeppelin performed an audit of the T-REX-Network/ONCHAINID repository at commit d2096509.

In scope were the following files:

 contracts
├── Identity.sol
├── IdentityUtilities.sol
├── KeyManager.sol
├── SmartAccount.sol
├── factory
│   ├── IIdentityFactory.sol
│   └── IdentityFactory.sol
├── interface
│   ├── IClaimIssuer.sol
│   ├── IERC734.sol
│   ├── IERC735.sol
│   ├── IIdentity.sol
│   ├── IIdentityUtilities.sol
│   └── IKeyExecutor.sol
├── libraries
│   ├── Errors.sol
│   ├── FormatResolver.sol
│   ├── Hashing.sol
│   ├── IdentityTypes.sol
│   ├── KeyPurposes.sol
│   └── KeyTypes.sol
├── modules
│   ├── claims
│   │   └── EASClaimIssuer.sol
│   ├── executors
│   │   ├── KeyApprovalModule.sol
│   │   └── RecoveryModule.sol
│   └── validators
│       ├── ERC734Validator.sol
│       └── ERC7579Validator.sol
├── proxy
│   └── IdentityUtilitiesProxy.sol
├── reputation
│   ├── IReputationRegistry.sol
│   └── ReputationRegistry.sol
├── storage
│   └── Structs.sol
└── vendor
    └── eas
        └── IEAS.sol

The deployment script (scripts/DeployOnchainID.s.sol) and its AccessManager policy table were not in scope and were consulted only as the client's statement of the intended configuration. The RecoveryModule contract is in scope as an integration wrapper, but the upstream ERC7579SocialRecoveryExecutor it inherits was not reviewed, because the OpenZeppelin accounts source is not reachable in the audited checkout, where it is present only as a compile stub. Tests and third-party dependencies were not in scope.

Update: The fixes for the findings highlighted in this report have all been merged at commit 4fda205.

System Overview

ONCHAINID is the on-chain identity layer of the T-REX (ERC-3643) permissioned-token framework. Each participant holds an identity contract that records cryptographic keys and verified claims, and token contracts consult that identity to decide whether a wallet is eligible to hold a regulated asset. The audited revision reimplements the identity as a modular ERC-7579 smart account, which adds ERC-4337 account abstraction and, through ERC-7913, support for secp256r1, RSA, and WebAuthn signers in addition to ECDSA.

Keys and authorization: ERC-7579 has no concept of key purposes and defines its own execution path, while the ERC-734 model defines a purpose system and its own execute-and-approve path. The ERC734Validator module reconciles the two. It is at once the key registry that records which key holds which purpose, the validator that authorizes user operations by checking the signer and applying per-target scoping (a management key passes any target, self-targeted and factory-targeted calls require a management key, and other external targets require an action key), and the claim registry whose EIP-712 digest-keyed revocation is designed to prevent the resurrection of a revoked claim through signature malleability. The account contract re-applies the same restrictions on its own execution path, and the legacy execute-and-approve queue is provided by a separate KeyApprovalModule.

Identity binding: The IdentityFactory deploys each identity as a CREATE3 proxy backed by a shared upgradeable beacon and maintains the wallet-to-identity bindings that token contracts rely on for eligibility. Wallets are stored as ERC-7930 interoperable-address envelopes so that EVM, ERC-7913, and non-EVM signers share one keyspace. A local wallet binds by submitting an EIP-712 signature that proves control, while a non-EVM wallet proves control on its native chain and relays that proof through an authenticated ERC-7786 gateway message, which the target identity finalizes. Bindings are sticky, and revocation is terminal.

Supporting components: A reputation registry gates trusted-issuer claim writes, an EAS adapter resolves Ethereum Attestation Service attestations as claims, a recovery module wraps an external social-recovery executor, and a utilities registry maps claim topics to schemas for off-chain consumers. Access control uses the OpenZeppelin AccessManager for the factory, the reputation registry, and the EAS adapter.

Security Model and Trust Assumptions

The model below describes the system as reviewed, at the in-scope commit d2096509, and is the baseline this report's findings are written against rather than a description of the current deployment. Several of the behaviors noted here were changed by the remediations merged at 4fda205 (see the Scope update above).

Authorization is expressed through ERC-734 key purposes. A management key fully controls an identity, including module installation, key rotation, and wallet binding. An action key is a lower-privilege signer, and the boundary between an action key and a management key is the principal property this report examines. Claim-related and proposer purposes carry narrower authority.

Two assumptions are load-bearing. First, an identity's module set is chosen by whoever deploys it: the initializer requires only that at least one validator or executor be installed, and no canonical module configuration is enforced on-chain, so several safety properties depend on how an identity is assembled rather than on invariants the contracts guarantee. Second, the reach of the factory's third-party creation and linking entry points depends on the AccessManager policy configured at deployment, which is defined in the out-of-scope deployment script. The contracts authenticate a caller against a role, but whether that role is public or restricted is a deployment decision.

Privileged Roles

Identity owner (management key): controls the identity, including module installation and removal, key rotation, and the binding and revocation of wallets through the factory.

AccessManager admin: governs the factory, the reputation registry, and the EAS adapter. The admin configures the per-identity-type creation policy, the trusted cross-chain gateways, the reputation scores, and the EAS topic-to-schema mappings and attester allowlist. The admin also initializes and upgrades the beacon that every identity proxy points to, and can therefore replace the implementation of all deployed identities at once and, at the reviewed commit, without delay; the upgrade path was subsequently placed behind a dedicated timelocked role. This role starts in the deployer's control.

Utilities registry admin and topic manager: role-based accounts that curate the topic-to-schema catalog and can replace the implementation of the UUPS-upgradeable utilities registry. Both roles start with the admin chosen at initialization.

Identity-type creation roles: per-type roles that gate who may create an identity of a given type through the factory's third-party entry point.

Trusted claim issuers and EAS attesters: parties whose attestations are accepted as valid claims. They are trusted to issue and revoke attestations faithfully. At the reviewed commit, the attester allowlist is global, so each allowlisted attester is trusted for every configured topic; it was subsequently keyed per topic.

Cross-chain gateway: an ERC-7786 gateway trusted to deliver only authentic cross-chain link messages. A message admitted from a trusted gateway is treated as proof that the wallet authorized the link on its source chain.

Social-recovery executor: a module that, once installed and granted a management key, can replace the identity's management keys. Its logic is out of scope for this audit.

Critical Severity

Validator and Account Resolve the Same Batch Calldata to Different Targets

A signer holding only the ACTION purpose can grant itself a MANAGEMENT key on a default identity, with no victim interaction and no misconfiguration, because the validator that authorizes a user operation and the account that executes it can be made to read the same batch calldata as two different target sets. The _scopeAllows function and the _execute function both decode a user operation's executionCalldata with OpenZeppelin's ERC7579Utils.decodeBatch, which leaves per-entry pointer validation to Solidity's generated calldata-array accessor. That accessor bounds each pointer against calldatasize() of the enclosing frame, and the two frames differ: the account is entered through execute, where the payload is the whole calldata, while the validator is entered through validateUserOp, where the same payload sits 420 bytes into the re-encoded user-operation buffer (with an empty initCode).

A batch entry carrying an oversized, wrapping pointer therefore reads in-range garbage in the validator, where the _targetAllowed function accepts it as an ordinary external target that an ACTION key may call, and reads out of range in the account, where every head word is zero so the target reads as zero, which resolves to the identity itself. The entry's callData offset wraps to attacker-planted bytes in the same way, so the account makes a self-call with attacker-chosen calldata and satisfies the onlyManagerOrSelf gate on the addKey function. A holder of nothing but an ACTION key thereby grants itself MANAGEMENT, and installModule, uninstallModule and a nested self-execute are reachable the same way.

Consider having _scopeAllows decode the batch into memory, so that each entry's pointers are validated against the payload rather than against the enclosing frame, or bounding them against the executionCalldata slice before dereferencing batch[i].target.

Update: Resolved at commit eadb9f7 on PR39.

High Severity

Queued Approval Omits the Factory Guard That Auto-Approval Enforces

The factory's wallet-binding entry points, linkAccount, revokeAccount and confirmCrossChainLink, authorize on the calling identity in msg.sender and carry no notion of which ERC-734 purpose drove the call, so the protocol treats the factory as a management-grade target at every gate between a key and it. The user-operation path does so in _targetAllowed, which rejects the factory for any non-MANAGEMENT signer. The account-side check on the queue path, _isKeyAuthorizedToCallTarget, also classifies the factory as management-grade, but it is invoked on the caller's key, and when the dispatch arrives from KeyApprovalModule that caller is the module itself, whose own key is MANAGEMENT in the reference wiring. The account-side factory guard therefore passes regardless of the purpose that requested the call, so the effective gate has to live inside the module.

Only one of the module's two authorization surfaces carries it. _canAutoApprove refuses to auto-run a factory-targeted request for a non-MANAGEMENT key, while approve distinguishes only self-targeted calls, which demand MANAGEMENT, from every other target, which demands no more than ACTION, so the factory falls into the generic external branch. An ACTION key can therefore queue a factory call, watch auto-approval correctly refuse it, then approve its own pending request, which _runApproved dispatches through the account's executeFromExecutor under the module's MANAGEMENT key. Driven against revokeAccount on the identity's own MANAGEMENT wallet, this strips the identity of that wallet, and the owner cannot restore the link afterward even with a freshly signed LinkAccount authorization.

Consider deriving the required purpose from a single shared helper used by both _canAutoApprove and approve, with the identity and the identity factory both classified as MANAGEMENT targets, so the queued-approval path enforces exactly the same factory rule as auto-approval and the user-operation validator.

Update: Resolved at commit 16101bc on PR41.

Key Registry Not Recognized as a Privileged Call Target

ERC734Validator is the enshrined registry, reachable as registryModule(), and it writes keys under registries[msg.sender], so any call an identity dispatches to it mutates that identity's own key set. The user-operation path guards this explicitly, with the _targetAllowed function refusing the registry as a target, but the _canAutoApprove function refuses only the factory and defers the rest to the account's own-module check. That check, in the _authorizeCall function, derives from installation state, while the registry address is an immutable returned by the registryModule function, so the two coincide only because the reference module list happens to install the validator as an executor, an entry with a zero purpose that no code path otherwise uses.

Should a deployment omit that entry, a key holding only ACTION could queue an execution targeting the registry with addKey calldata, which would grant it MANAGEMENT and allow it to remove the original owner's key.

Consider having _authorizeCall treat registryModule() as an own-module target unconditionally, and refusing that target in _canAutoApprove as the factory is refused.

Update: Resolved at commit e8f1e79 on PR41.

Medium Severity

Last-Manager Guard Counts Module Keys That Cannot Sign

The removeKey function protects an identity from losing its administrators by requiring byPurpose[MANAGEMENT] to hold more than one entry. That set counts module keys as well: the initialize function registers a MODULE-type key for every module installed with a non-zero purpose, and the reference wiring grants KeyApprovalModule MANAGEMENT. Such keys cannot sign anything, since the _rawERC7579Validation function rejects any signer whose keyType is MODULE, for user operations and ERC-1271 alike. On a single-owner identity the tally therefore already reads two, so the owner can remove their own MANAGEMENT purpose and leave the identity with no party able to authorize a management operation again.

The factory's post-deployment check reads the same set, so an identity whose only MANAGEMENT holder is a module install entry passes it and can be minted in that state from the outset.

Consider gating both the guard and the factory's check on the presence of a MANAGEMENT key the validator would accept as a signer, rather than on membership of the purpose set, and applying the guard only when the key being removed is not itself a module key, so that module uninstall still succeeds.

Update: Resolved at commit 84cd5fd on PR44.

Cross-Identity Claim Writes Are Authorized Against the Validator Singleton Instead of the Issuer

The addClaimTo function verifies the claim and then calls addClaim on the target identity, naming the calling account as the issuer. That call originates from the shared ERC734Validator singleton rather than from the issuer identity, so the target's fallback appends the singleton as the ERC-2771 caller and the _requireClaimKey function asks whether the target granted a claim key to the singleton, never whether it granted one to the issuer.

Two consequences follow. A target that does what the NatSpec says and grants CLAIM_SIGNER to the issuer identity still sees addClaimTo revert with SenderDoesNotHaveClaimSignerKey. The grant that does unlock the flow is not issuer-specific: the singleton is the same contract behind every identity on the deployment, so one claim key granted to it admits every issuer whose own claim-status check passes, letting one identity write attestations into another's registry attributed to an issuer the target never authorized. That key can also arise without a deliberate decision, since the initialize function grants a MODULE-type key to any module install entry carrying a non-zero purpose, and the _keyHasPurpose function lets MANAGEMENT satisfy the CLAIM_SIGNER check.

Consider adding an explicit issuer-bound cross-identity path that authenticates the issuing identity and evaluates the target's claim-key policy against that identity, rather than against whichever caller the fallback forwards.

Update: Resolved at commit 5e04575 on PR45.

Installed Executors Cannot Be Reached by the Account, Leaving Their Self-Gated Functions Unreachable

The _authorizeCall function reverts with OwnModuleTargetBlocked whenever a dispatched call targets an installed executor or the fallback handler for the called selector, with no exemption for any caller, including a MANAGEMENT self-call. The rule is deliberate: as the comment on that branch records, module functions are meant to be reached through the account's fallback dispatch, which appends the real caller ERC-2771 style, whereas execute(module, ...) skips that append and would leave the module misreading its caller. The block reaches further than that rationale requires, however. Because execute and executeFromExecutor are the only routes into the _execute function, which is the account's only outbound path, the account cannot originate a call to one of its own executors, and any module function gated on msg.sender == account is beyond its reach.

The RecoveryModule contract is documented as installed exactly that way. Its account-gated members are therefore unavailable: the owner's unilateral veto over a pending recovery, and the guardian, threshold, weights, delay, and expiration setters, which remain at their install-time values for the life of the identity, so an untrusted guardian set cannot be rotated by the identity it protects. The repository's own test_accountSelfCancelStopsRecovery test covers the veto using vm.prank(address(aliceIdentity)), a caller the deployed account cannot produce.

The blocking behavior is a property of SmartAccount and was verified against this repository. The list of gated members was not: it belongs to the upstream ERC7579SocialRecoveryExecutor that RecoveryModule inherits from the private openzeppelin-accounts dependency at pin e4c2a32a, which is present in the audited checkout only as a 19-line stub carrying no recovery logic, so that list should be re-verified against the real pinned source.

Consider routing the owner-side veto and the recovery configuration through a dedicated account entry point that reaches the module with the account as msg.sender, while keeping the block in place for ordinary dispatched calls.

Update: Resolved at commit 6d4814b on PR46.

The Factory's Post-Deployment Key Check Is Answered by a Caller-Supplied Fallback Handler

After CREATE3 deploys the proxy, _doCreateIdentity asserts the identity's shape before recording it as factory-deployed: it calls getKeysByPurpose for the MANAGEMENT purpose through the IERC734 interface and requires the returned array to hold at least one entry, reverting with NoManagementKeyInKeys otherwise. The account implements no ERC-734 getter of its own; those selectors are served by a fallback handler that Identity.initialize registers from the caller-supplied _modules array, and the only constraint on that array is that it contain a validator or an executor. The deployer therefore chooses which contract answers getKeysByPurpose, so the post-deploy check asks a contract the same deployer submitted, in the same call, whether the deployer's identity is well-formed.

A handler that returns a one-element array satisfies the check while the enshrined ERC734Validator registry, read directly, holds no MANAGEMENT key for the identity. The deploy runs to completion and the wallet is linked, and the state is permanent: addKey reverts with SenderDoesNotHaveManagementKey for every external caller, _targetAllowed rejects a non-MANAGEMENT signer for self-targeted, validator and factory calls so an ACTION key can neither promote itself nor reach revokeAccount, and _linkAccount sticky-binds the deployer's wallet to that unmanageable identity for good.

The isFactoryIdentity flag the identity earns is load-bearing elsewhere. ReputationRegistry.reputationOf gates the default reputation tier on it, and EASClaimIssuer._resolve treats it as proof that an attestation recipient is not an arbitrary contract posing as the identity, both resting on a key-shape invariant the factory does not in fact verify. Identity.supportsInterface likewise advertises IERC734 conformance for a getter surface the deployer selected, so any integrator reading key state through the identity address rather than the registry module reads deployer-controlled answers for the life of the identity.

Consider performing the factory-side integrity check against the enshrined registry that the identity's registryModule view returns, rather than through the account, so the assertion cannot be routed through caller-installed dispatch, and having Identity.initialize reject fallback installs for the ERC-734 getter selectors that point anywhere other than the registry module.

Update: Resolved at commit 248e116 on PR47.

Non-Canonical ERC-7930 Envelopes Give One Wallet Multiple Registry Keys

The factory identifies every wallet by hashing its ERC-7930 envelope: _walletKey returns keccak256 over the raw account bytes with no normalization, and the registry's collision guarantees, namely sticky binding, terminal revocation and one wallet per key, all depend on that hash being a stable identifier. The encoding is not canonical, however: linkAccount extracts the signer with InteroperableAddress.parseV1Calldata, whose NatSpec states that trailing bytes after a valid v1 encoding are ignored, so the same decoded address can correspond to multiple distinct input byte strings. Appending two zero bytes to a wallet's canonical envelope yields a blob that decodes to the identical signer but whose keccak256 differs, so _walletKey returns two different keys for one wallet, and the owner can freely produce a valid LinkAccount signature for each encoding.

Three invariants break as a result. Sticky binding: _linkAccount rejects re-binding only when entry.identity is already set, so a padded encoding is a fresh entry that can be bound to a different identity, and one wallet resolves to two identities through getIdentity and appears twice in getAccounts. Terminal revocation: the WalletAlreadyRevoked guard fires only for the exact key revoked, so after revokeAccount retires the canonical envelope the wallet re-links through a padded one, defeating the guarantee that a revoked wallet can never be relinked that the compliance model rests on. And wallet uniqueness: the registry the T-REX modules treat as the source of truth reports one signer as an unbounded number of independent wallets, while the client's own test_envelopes_evmAndNonEvmAreDistinct frames hash distinctness as a safety property and never exercises two encodings of a single wallet.

Consider deriving the wallet key from the canonical decoded components rather than the raw bytes, hashing keccak256(abi.encode(chainType, chainReference, signer)) or re-encoding the parsed triple through formatV1 before hashing, so every encoding of one wallet collapses to a single key, and applying the same normalization on the cross-chain path in _processMessage. Alternatively, reject non-canonical input outright by requiring the consumed envelope length to equal the input length, so that linkAccount, revokeAccount and _processMessage all refuse trailing bytes.

Update: Resolved at commit dced9d4 on PR48.

Signature-Based Linking Does Not Constrain the Envelope's Chain Type or Reference

The IIdentityFactory interface documents two disjoint proof-of-control routes: EVM and ERC-7913 signers prove control through an on-chain signature check, while non-EVM wallets prove control on their native chain and relay that proof through an ERC-7786 message. The linkAccount function does not enforce the split. It parses the ERC-7930 envelope but keeps only the signer field, discarding the chain type and reference that say which chain the wallet lives on, and then admits the binding on whatever SignatureChecker.isValidSignatureNow accepts over the LinkAccount digest.

That helper dispatches on signer length: twenty bytes is an EOA or an ERC-1271 wallet, and anything longer is treated as ERC-7913, where the leading twenty bytes name the verifier that judges the remaining key. For every envelope whose address field exceeds twenty bytes, the proof is therefore self-certifying, since a caller can deploy a permissive verifier that returns the success selector for any input, place it in those leading bytes, and link with a dummy signature. Because the chain type is never read, that envelope may carry a non-EIP-155 tag and a foreign chain reference and still bind through the path reserved for EVM and ERC-7913 signers. The caller needs a MANAGEMENT key on the identity and binds the wallet to itself, so this is registry pollution rather than an outside takeover, but it leaves a permanent, enumerable account for which no control was proven, indistinguishable through getAccounts and getIdentity from a wallet that proved control on its own chain. The repository's own test_linkAccount_nonEvmEnvelopeRejected test asserts that such an envelope is rejected, and holds only because the thirty-two-byte signer it chooses begins with twenty bytes that carry no code.

Consider constraining the signature path to the chains it can actually verify, requiring the envelope's chain type to be EIP-155 and its address field to be a canonical twenty-byte EVM address or a recognized ERC-7913 shape, and routing foreign wallets exclusively through the confirmCrossChainLink function, which carries native-chain proof. Where ERC-7913 signers are accepted, consider binding the permitted verifiers to a trust list rather than accepting a caller-embedded verifier as its own attester.

Update: Resolved at commit 0587147 on PR49.

Claim Digests and Revocation Records Are Recomputed From the Issuer's Live EIP-712 Domain

The _getClaimDigest function rebuilds the domain separator from the issuer's eip712Domain() on every call, and nothing persists the result: the _getClaimStatus function verifies each stored signature against a freshly computed digest, and the removeClaim function keys its revocation record on one.

Identities are beacon proxies taking their domain from EIP712("OnchainID", "1") in the Identity constructor, while the version function returns "3.0.0". An upgrade that changes the EIP-712 name or version, for instance to reconcile that mismatch, shifts the domain for every identity in the deployment at once, with two effects: stored signatures no longer verify, so valid claims read as BadSignature, and existing revocation records are keyed under the old domain, so revoked claim content can be re-added.

The length of those strings is load-bearing in a way nothing records. OpenZeppelin's EIP712 keeps the name and version in immutables when each fits a ShortString and in a storage fallback otherwise, and the constructor runs on the implementation rather than on any proxy. "OnchainID" and "1" fit, so every proxy reads the immutable and agrees with the implementation. A replacement name over 31 bytes would instead be written to the implementation's storage, leaving each proxy to read its own empty slot, so the domain would degrade to an empty name with no revert and no event, taking every claim digest and every revoked-digest key with it.

Consider persisting each claim's digest at insertion time instead of recomputing it on read, so that both claim validity and revocation are independent of the issuer's current domain, and documenting the EIP-712 name and version as frozen across upgrades, with the 31-byte ShortString limit called out explicitly since exceeding it empties the domain rather than merely changing it.

Update: Acknowledged, not resolved.

The client stated:

Closing per the team decision: M-07 is marked acknowledged, not resolved. Domain changes invalidating prior signatures is standard EIP-712 behavior, and the trigger requires deliberately editing the EIP712("OnchainID", "1") literal in a beacon upgrade — the rule is to never change the domain on upgrade. Agreed with the points raised in review.

One part of this PR was independent of M-07: the try/catch around eip712Domain(), which is what keeps removeClaim working for issuers that don't implement ERC-5267 (the EAS adapter). That fix now lives in #59 (L-08) on its own branch off develop, so nothing is lost by closing this.

We agree this is a reasonable acknowledgment for a design that treats the EIP-712 domain as immutable. Claim validity and revocation are recomputed from the issuer's live domain rather than a persisted digest, which is safe as long as that domain never changes, and you have pinned the domain version to "1", kept it separate from the identity release version (N-06, so a release bump creates no pressure to change it), and documented in Identity that it must not change.

Two points for the record:

1. This is a mitigation, not an elimination. The guarantee now rests entirely on the invariant that the EIP-712 name ("OnchainID") and version ("1") are never altered in any future implementation. If an upgrade ever changes either, every stored signature reads as BadSignature, and revocation records keyed under the old domain stop matching, so previously revoked claim content can be re-added. We recommend keeping that invariant called out wherever upgrades are described.

2. One specific failure mode belongs in that note. OpenZeppelin's EIP712 holds the name and version as ShortString immutables only while each fits 31 bytes, so a replacement name above that limit empties the domain silently, with no revert and no event, taking every claim digest and revocation key with it. The invariant should therefore also state that any future name stays within the 31-byte ShortString limit.

With those noted, we are marking M-07 Acknowledged.

Cross-Chain Link Confirmation Skips the Validations the Local Path Enforces

Wallet bindings reach the registry through two paths that converge on _linkAccount, but only the EVM path validates its inputs. linkAccount parses the ERC-7930 envelope with InteroperableAddress.parseV1Calldata, reverting on anything malformed, and then rejects ASSET and SMART_CONTRACT targets with CannotLinkToAssetIdentity by reading getIdentityType on msg.sender. The cross-chain half does neither: confirmCrossChainLink matches the caller against the staged proposal and calls _linkAccount, and the staging step _processMessage checks only sender and wallet equality, expiry, factory membership and the absence of a prior entry, treating the wallet as opaque bytes, even though the interface documents confirmation as linking the wallet with the same sticky-binding rules as any EVM-side link.

Three checks are dropped as a result. An ASSET identity can accumulate wallets beyond the token auto-linked at _doCreateIdentity, and because revokeAccount rejects ASSET and SMART_CONTRACT callers the extra wallet stays active for the life of the identity. An arbitrary byte string that linkAccount would reject becomes a live, enumerable wallet record that makes any consumer parsing getAccounts revert over the whole set rather than one element. And because _processMessage never parses the envelope, a sender of the form formatEvmV1(block.chainid, wallet), byte-for-byte the shape the local entry points build, stages a pending link for a local EVM address that the named identity can finalize without that address ever signing the LinkAccount digest or consuming its nonce, after which sticky binding locks the real owner out with WalletBoundToAnotherIdentity. All three require a trusted gateway to deliver the proposal and the named identity to confirm with a MANAGEMENT key, and the same-chain facet additionally needs a gateway that emits a same-chain sender a faithful relayer would not produce, so it is defense in depth rather than a path open to an unprivileged caller.

Consider moving the shared preconditions into _linkAccount so both entry points inherit them, parsing the envelope and rejecting ASSET and SMART_CONTRACT targets there, with a carve-out for the factory's own auto-link in _doCreateIdentity, and parsing the envelope in _processMessage so a malformed proposal fails on delivery rather than on confirmation, additionally rejecting proposals whose chain reference equals block.chainid, since a wallet on this chain already has a signature-checked route through linkAccount.

createIdentityFor Binds a Wallet Without Proof of Control and Does Not Tie Management to the Account

The createIdentityFor function takes an arbitrary _account, gates the caller on a per-type role through _checkTypeRole, and passes _account to _doCreateIdentity, which auto-links it with _linkAccount. The signature-checked linkAccount admits a wallet only against a LinkAccount signature that proves control, and the self-service createIdentity binds the caller's own address, but this third-party path takes no signature and no other proof that the caller controls _account.

The identity's keys carry the same gap. _keys comes from the caller, and Identity.initialize forwards each entry to the registry as supplied — deriving the key hash from the caller's signerData and never checking any key against _account. The only post-deployment assertion is that at least one MANAGEMENT key exists, never that any MANAGEMENT key corresponds to _account. A caller can therefore make its own key the sole MANAGEMENT key, name the victim's wallet as _account, and have the factory bind that wallet to an identity the caller alone controls.

The binding is permanent for practical purposes. _linkAccount refuses to move a wallet already bound elsewhere and rejects one that was ever revoked, the factory exposes no unlink primitive, and revokeAccount is terminal and refuses ASSET and SMART_CONTRACT identities, so the real owner cannot later link the wallet to its own identity and is met with WalletBoundToAnotherIdentity. It holds on the canonical envelope the factory itself writes, since createIdentityFor formats the wallet through formatEvmV1; because the lookup key is a bare keccak256 of the raw envelope with no canonicalization, a padded re-encoding of the same wallet hashes to a different slot and produces a second, conflicting binding rather than a clean recovery — that non-canonical keying is tracked as a separate finding.

Eligibility follows the binding. EASClaimIssuer._resolve resolves an attestation's recipient wallet to an identity through getIdentityIncludingRevoked, so an attestation issued to that wallet — provided it matches the adapter's configured schema, comes from an allowed attester, and is neither revoked nor expired — validates as a claim on the caller-controlled identity, which isFactoryIdentity still reports as factory-deployed to every consumer that trusts that flag. A SMART_CONTRACT subject has no equivalent of generating a fresh address, since its identity stands for a contract that already exists; a PUBLIC_AUTHORITY subject (a regulator, court, or government issuer) is an off-chain institution that need not be a deployed contract, but it likewise has no way to prove control on this path.

Whether an unprivileged caller can reach this depends on the deploy-time AccessManager policy that _checkTypeRole reads. That policy lives in the out-of-scope deploy script, which grants PUBLIC_ROLE to the EOA-shaped types (INDIVIDUAL, CORPORATE, IOT, AI_AGENT) and also to the contract-shaped SMART_CONTRACT and PUBLIC_AUTHORITY — the latter with selfDeployable set to false, so no account may create those identities for itself, yet any account may create one for an arbitrary third party. Restricting createIdentityFor to trusted issuers narrows the exposure to those issuers, but it does not close the gap, because the code proves no control over _account and does not bind the identity's management authority to it.

Consider requiring proof of control over _account on the third-party path — either a LinkAccount-style signature bound into the deployment, or a guarantee that _account is the identity's sole MANAGEMENT authority, rejecting additional MANAGEMENT keys and MANAGEMENT-purpose module installs rather than merely requiring _account to appear among the keys. Where the subject cannot sign, such as ASSET and SMART_CONTRACT, restrict createIdentityFor for those types to trusted roles rather than opening them to PUBLIC_ROLE. This was reproduced against the pinned commit, though through a test helper whose type policies are more permissive than scripts/DeployOnchainID.s.sol, so the exact reachable set should be confirmed against the deploy script's policy table before sign-off.

Update: Resolved at commit 9a0a011 on PR52.

Low Severity

Claim Path and Key Path Use Different Definitions of a Valid Signature

The _verify function recovers a 20-byte signer with ECDSA.tryRecover and falls through to ERC-1271 only when recovery does not match, deliberately avoiding SignatureChecker's signer.code.length check because it conflicts with ERC-7562 bundler rules. The _getClaimStatus function calls SignatureChecker.isValidSignatureNow instead, which drops the ECDSA branch entirely once the signer address carries code, so one CLAIM_SIGNER key in one registry is judged by two different rules.

An EIP-7702 delegation on a claim-signer EOA gives that address code, and the stored ECDSA signature is then routed to the delegate's isValidSignature, which may not return the ERC-1271 magic value for a raw signature blob. A delegate that recovers ECDSA against the delegating account still accepts it and the claim keeps verifying; one that implements no isValidSignature, or that expects a wrapped signature format, does not. _getClaimStatus then reports BadSignature for an unrevoked, unexpired claim whose signature the key path still accepts, so claim validity depends on mutable external state that neither the issuer nor the holder controls, and can move in either direction as the delegation changes. An ERC-3643 deployment reading claim validity as an eligibility gate would see every claim backed by that signer drop on an unrelated wallet upgrade.

Consider routing _getClaimStatus through _verify, so that a single definition of a valid signature governs the whole module.

Update: Resolved at commit d346e8e on PR54.

Introspection Reports Static Support Rather Than Live Module Wiring

The account answers introspection from constants, so its answers can disagree with what the installed modules actually serve:

• The supportsInterface function returns true for IERC734, IERC735 and IIdentity regardless of wiring. Its NatSpec justifies this on the grounds that the interface contract is still honored at runtime, which stops being true once the ERC-735 fallback handler is uninstalled and getClaim begins to revert.
• supportsExecutionMode, inherited from OpenZeppelin's ERC-7579 base and not overridden in this repository, advertises CALLTYPE_DELEGATECALL, which the _execute function rejects.
• The advertised ERC-734 identifier is repository-specific: the IERC734 interface declares only the six registry methods, so its interfaceId omits the execute and approve selectors a canonical ERC-734 consumer folds into the identifier.

Consider gating supportsInterface on the relevant handler being installed, overriding supportsExecutionMode to match what _execute accepts, and reconciling the advertised ERC-734 identifier with the selectors the account really serves.

Update: Resolved at commit d49337d on PR80.

Unbounded Dynamic Fields in the Key and Claim Registry

The _addKey function enforces only a minimum signer length and the _addClaim function enforces nothing, so five stored fields have no upper bound:

• signerData and clientData, written by _addKey
• signature, data.payload, and uri, written by _addClaim

Because the removeClaim function re-reads and re-emits the claim's dynamic fields before deleting them, removal costs more than the write at large sizes, so a sufficiently large claim may not be removable within a block. One oversized claim also breaks any aggregation covering its topic for the whole identity.

Consider capping the five fields, sized against the signer types the deployment intends to support.

Update: Resolved at commit fc19ae6 on PR55.

scheme and uri Are Not Covered by the Claim Signature

The _addClaim function keys a claim on (issuer, topic) and overwrites the stored record with the caller-supplied scheme and uri, gated only by the issuer's isClaimValid, while the digest built by the _getClaimDigest function covers only the topic, the subject and the claim data. Re-presenting the issuer's own unchanged signature and data alongside a different uri therefore succeeds, so a holder-side claim key can repoint a KYC claim at an attacker-controlled document while the record remains attributed to the legitimate issuer and continues to validate.

The _buildClaimInfo function returns that uri alongside isValid: true, so an off-chain consumer fetching the referenced document reads holder-controlled content presented as issuer-attested.

Consider binding scheme and uri into the signed digest, or rejecting a re-add that changes either field while the existing claim is still valid.

Update: Resolved at commit 03f7a15 on PR79.

Claim Events Carry No Subject and the Declared ERC-734 Events Are Never Emitted

ERC734Validator is a single module shared by every identity that installs it, and it emits the ERC-735 lifecycle events itself.

• The events declared on IERC735 carry no identity or subject, and the _addClaim function derives claimId from the issuer and topic alone. The same issuer's claim on the same topic, written to two different identities, therefore produces byte-identical ClaimAdded logs from the same emitter.
• The six events declared on IERC734 are emitted nowhere. Every emission resolves to a same-named local event carrying an additional address indexed account, declared in the validator for the key events and in KeyApprovalModule for the execution events, whose topic hash differs from the declared one. IKeyExecutor nonetheless states that consumers and indexers continue to interpret these as ERC-734 calls.

An integrator building against the declared event ABI captures no key or execution lifecycle at all, and cannot attribute a claim change to a specific identity from logs alone.

Consider including the subject identity as an indexed field in the claim events, mirroring the address indexed account the key events already carry, and either emitting the canonical ERC-734 events or removing their declarations and documenting the account-carrying variants as the real ABI.

Update: Resolved at commit 66f3275 on PR56.

Uninstalling One Fallback Selector Strips All of the Module's Purposes

ERC-7579 registers fallback handlers per selector, so a module serving several selectors is installed once per selector and may additionally be installed as an executor, while the initialize function grants that address a single MODULE-type key backing every one of those installs. The _uninstallModule function does not distinguish between them: whichever install is being removed, it snapshots every purpose the module's key holds and removes all of them before delegating to the base uninstall.

Removing a single read-only handler such as getCurrentNonce therefore revokes the module's MANAGEMENT purpose and, with its last purpose gone, deletes the key entirely, while the module remains an installed executor and the handler for its other selectors. Its later dispatches are then refused by the _authorizeCall function with ExecutorPurposeNotAuthorized until a MANAGEMENT holder re-grants the purpose. On an identity where the module's key is the only MANAGEMENT holder the strip instead reverts through the guarded removeKey, so the handler cannot be uninstalled at all.

Consider stripping purposes only once the module has no remaining install of any type, or, if enumerating the selector map is impractical, skipping the purpose removal for MODULE_TYPE_FALLBACK uninstalls and requiring an explicit removeKey instead.

Update: Resolved at commit 2fe620f on PR57.

Re-Added Claim Cannot Be Removed a Second Time

The removeClaim function derives an EIP-712 digest over the issuer, subject, topic, and claim data, then marks it revoked in both the holder's and the issuer's registry. Only the issuer-side record is read by the _getClaimStatus function to reject a repeated submission, and only when the issuer is an identity backed by this validator. For other issuers the identical claim can be added again, and a second removal then recomputes the same digest, finds it already marked, is rejected by the holder-side guard, and reverts, leaving the claim permanently active.

Consider dropping the holder-side guard, or keying the re-removal check on claim existence, which the function already establishes by rejecting an unregistered topic.

Update: Resolved at commit 1465732 on PR60.

Claims From Issuers Without an EIP-712 Domain Cannot Be Removed

The removeClaim function computes the claim digest before deleting the local record, and the _getClaimDigest function obtains the issuer's EIP-712 domain by calling eip712Domain() on it. Nothing at insertion requires the issuer to implement ERC-5267, since the _addClaim function delegates validation to the issuer's own isClaimValid and stores the claim without reading the domain.

The shipped EASClaimIssuer contract exposes no domain method, so the call reverts and a claim mirrored from an EAS attestation can be added but never deleted, including after the attestation has been revoked at the source. That contradicts the adapter's documented lifecycle, in which the attester revokes on EAS and the holder removes the local record through removeClaim.

Consider making local deletion independent of the issuer's EIP-712 domain, or adding an explicit adapter-compatible removal path; at minimum, probing the issuer's capabilities before attempting the digest computation so that a supported claim type cannot become undeletable.

Update: Resolved at commit 067f661 on PR59.

Initialization Discards KeyParam.keyHash While the Deploy Salt Commits to It

Every key handed to the factory is a Structs.KeyParam carrying a keyHash field documented as keccak256(signerData). The registry's convenience writer KeyManager._addKeyWithData enforces that commitment by rejecting any key whose declared hash does not equal the hash of the supplied signer data, and its NatSpec states it is used by the external entry point and the initialization path. The initialization path does not in fact use it: Identity.initialize forwards only the signer data, client data, purpose and key type of each key and never reads keyHash, and the registry re-derives the hash from the signer data before writing, so the advertised guard never runs on this path.

The discarded field is not inert, because the factory's CREATE3 deploySalt commits to it: the deployed address depends on keyHash while the key the identity actually registers depends only on the signer data. A deployer can vary keyHash freely, moving the deployed address without changing the registered key, so an off-chain consumer that predicts the address and reads the salt's keyHash commitment as a statement of which key the identity holds is misled, with nothing on-chain flagging the divergence. No mismatched key is ever written, which bounds the impact, but the documented API guarantee is violated and the salt's apparent commitment is not the property it advertises.

Consider either routing initialization through _addKeyWithData so the same hash check applies on this path, or removing keyHash from Structs.KeyParam entirely and correcting the NatSpec, so the deploy salt commits to exactly the key material the identity will register.

Update: Resolved at commit 68fedff on PR58.

EASClaimIssuer Resolves Attestations as Valid for the Zero Identity

EASClaimIssuer answers IClaimIssuer queries by translating an EAS attestation into a claim status: isClaimValid delegates to _resolve, which, after confirming schema, attester, revocation and expiry, decides whether the attestation belongs to the queried identity. The self-recipient branch is guarded by isFactoryIdentity, but the linked-wallet branch compares the identity the factory reports for the recipient against the queried identity without first requiring that a link exists. For a recipient wallet not linked in the factory the lookup returns address(0), so a query made for the zero identity satisfies the equality and the adapter returns Valid.

Any attestation from an allow-listed attester under a schema-mapped topic therefore resolves as a valid claim for address(0) whenever its recipient is an unlinked wallet, even though the zero identity is not a factory identity and holds no claims. The practical exposure is a compliance path that resolves an unregistered wallet to address(0) and forwards it straight into isClaimValid, where the check passes instead of failing closed. This is a defensive gap and an IClaimIssuer contract violation rather than an attacker-driven escalation.

Consider rejecting a zero identity up front in _resolve by returning NotIssued, or requiring the linked address to be non-zero before the equality check, so an unlinked recipient can never match the zero identity.

Update: Resolved at commit 4fab9cf on PR61.

The CREATE3 Deploy Salt Does Not Bind the Account That Is Auto-Linked

The factory's shared creation path derives the CREATE3 deploySalt from the identity type, the caller-chosen salt string and the hashes of the keys and modules, but not from the account it is about to auto-link, and that salt is the only input to the CREATE3 address besides the factory itself. createIdentityFor lets the caller name any account and then binds it through _linkAccount. A front-runner who observes a pending creation and replays its four public arguments therefore deploys the identity at the exact address the victim expected, carrying the victim's MANAGEMENT key, but with the front-runner's own wallet auto-linked as the identity's first account. For a self-deployable type the replay needs no role through createIdentity.

The binding is sticky, since _linkAccount offers no unlink short of a terminal revoke. The victim keeps control of the identity, but getIdentity permanently resolves the attacker's wallet to the victim's identity, the exact lookup that ERC-3643 eligibility is decided on, so the attacker's wallet inherits the victim's KYC, and the only cure is a revoke that permanently burns the attacker's address for the whole deployment. This is the opposite direction from the separate createIdentityFor squatting finding, and it needs a different fix, since requiring a LinkAccount signature does not help when the front-runner binds a wallet it controls and can sign for.

Consider binding the auto-linked account into deploySalt, so that changing that account changes the deployed address and the front-run deploy lands at a different address than the victim's own transaction.

Update: Resolved at commit f532ef1 on PR62.

ERC-2771 Caller Recovery Mis-Attributes Every Call That Does Not Reach the Identity Directly

The externalized ERC-734 modules recover the off-chain caller from the ERC-2771 trailing 20 bytes the account's fallback dispatch appends: both KeyApprovalModule._msgSender and ERC734Validator._msgSender read the last 20 bytes whenever the calldata is at least that long. That tail is the real caller only on the direct path, where a key holder calls the identity at an ERC-734 selector and the fallback appends its immediate caller; as SmartAccount._authorizeCall notes, any dispatch that does not go through that fallback skips the append, so the recovered value is whatever the immediate on-chain caller happened to be. Two paths are mis-attributed as a result: execute auto-approves a self-targeted addClaim for a CLAIM_SIGNER caller and dispatches it immediately through executeFromExecutor, so the account self-calls addClaim and _requireClaimKey recovers the account itself, which holds no claim key and reverts; and a call relayed through an intermediary such as the EntryPoint recovers that intermediary, which likewise holds no key.

The impact is limited and fails closed, because in every case the mis-recovered caller is less privileged than the true one, so the effect is a refusal or a silently failed dispatch rather than an escalation. What is lost is functionality: the self-targeted claim auto-approval branch can never succeed, and relayed or meta-transaction routes into execute, approve and addClaim are unusable.

Consider threading the original caller through the queue's own dispatch rather than re-deriving it from the appended tail after a self-call, and either documenting that these entry points are reachable only by a direct call from the key holder or removing the auto-approval branch that can never run.

Update: Resolved at commit c70be18 on PR63.

Thank you for the fix.

The code change resolves the broken self-targeted auto-approval path, and no further behavioral change is required provided that direct-call-only access is intentional.

As a minor, non-blocking recommendation, please expose this limitation in the public-facing NatSpec or integration documentation: execute, approve, addClaim, and removeClaim require a direct call through the identity fallback and are not supported through EntryPoint, account self-calls, or other meta-transaction routes.

The current implementation comments explain this, but making it visible to integrators would prevent ERC-4337 users from assuming these legacy entry points are UserOp-compatible.

Queued Executions Carry No Proposer or Expiry and Survive Key Revocation

A queued request in the execution module stores no record of who queued it and no time bound: the Execution struct holds only the target, value and calldata together with its approved and executed flags. execute derives the proposer only to gate proposing and to populate the event and never persists it, approve authorizes the approver's key at approval time and never re-checks the proposer, and onUninstall is a no-op, so uninstalling the module leaves the per-account queue intact in shared storage. A pending entry therefore outlives the key that queued it, survives an uninstall and reinstall of the module, and never expires.

The impact is limited and fails closed, because executing a stale entry still requires a live ACTION or MANAGEMENT approver at approval time, so no privilege is gained and no arbitrary address can inject a request. The residual is that an execution the original proposer can no longer endorse, since that key was revoked, can still be dispatched later, with no on-chain record of who proposed it.

Consider persisting the proposer and an expiry with each queued request, rejecting approval of an entry whose proposer no longer holds the authorizing purpose, and clearing pending entries in onUninstall so a queue cannot be resurrected across a reinstall.

Update: Resolved at commit 5212b48 on PR64.

Thanks for the follow-up. Rejection no longer requires a live proposer (only an authorized ACTION/MANAGEMENT approver), so a stale entry can be closed, and onUninstall now bumps a per-account firstValidId floor that voids the pending queue across a reinstall. Together with the proposer re-check on approval, the key-revocation and resurrection paths are closed.

One item remains unimplemented: an autonomous time-based expiry. A request whose proposer stays authorized, that is never rejected and outlives no uninstall, still has no time bound. Given the severity and that stale entries can now be closed on demand and voided on uninstall, we accept this as a minor residual and consider the finding resolved.

A Failed Dispatch Is Recorded as Approved and Executed and Cannot Be Retried

The _runApproved function writes executed = true and approved = true before it dispatches, then wraps the dispatch in a try/catch that emits ExecutionFailed and returns false rather than reverting. Neither write is undone, so a request whose dispatch failed is left carrying exactly the flags of one that succeeded. The Execution struct has no field recording the outcome and getExecutionData returns that struct verbatim, so nothing on-chain separates the two states: a failed request is byte-identical to a successful one in every field.

The behavior has several independent triggers. A dispatch fails when the target reverts, when the identity's balance is below the request's value, when the target is one of the account's own modules and SmartAccount._authorizeCall refuses it, and when the module's purpose has been revoked, whether by the purpose strip a fallback uninstall performs or through the MANAGEMENT-gated KeyManager.removeKey with no uninstall involved. Both entry points reach the helper, since execute auto-runs through it and approve routes to it as well. The repository's own test_execute_autoApprove_targetReverts_ethStaysInIdentity test already drives the path, asserting only on balances and never reading the flags.

The impact is off-chain and fails closed. Nothing in contracts/ reads getExecutionData or the struct, no funds move, and no privilege is gained; the cost falls on an integrator or operator reading queue state, who sees a request that never ran reported as approved and executed. The residual that is not merely cosmetic is that the request is terminal: approve rejects any id already carrying executed, and nothing resets the flag, so a dispatch that failed for a transient reason cannot be retried once the cause is cleared and the only way forward is a fresh execution id.

Consider recording the dispatch outcome in a third field on Execution, surfaced through getExecutionData, rather than letting executed stand for both attempted and succeeded. Moving the two writes after the try is not a safe alternative: they are what stops the dispatch target re-entering approve on the same id.

Update: Resolved at commit dac0be4 on PR65.

The EAS Adapter's Attester Allowlist Is Global Rather Than Scoped Per Topic

EASClaimIssuer binds trust along two axes, but only one of them is topic-scoped. _resolve accepts an attestation for a queried topic when its schema matches the topic's binding and its attester is allowlisted: attestation.schema == getSchemaForTopic(topic) together with getIsAttesterAllowed(attestation.attester). The schema binding is per topic (_schemaOf), but the attester allowlist is a single global mapping (_isAttesterAllowed, set by setAttester) with no topic or schema parameter.

The consequence is that allowlisting an attester for one topic implicitly authorizes it for every configured topic. On an adapter instance serving more than one topic, any allowlisted attester can satisfy any other configured topic by producing an attestation under that topic's bound schema, since nothing in _resolve ties an attester to the topic it was trusted for. The per-topic schema check closes only the schema axis: it stops an attestation made under topic A's schema from being replayed against topic B, but not an attester trusted for topic A from attesting under topic B's schema, because any address may attest under any schema on EAS and the attester gate is global. This diverges from the ERC-3643 trusted-issuer model the adapter says it mirrors, since the canonical TrustedIssuersRegistry scopes each issuer to an explicit list of claim topics.

The exposure is bounded, which holds this at Low: attesters are admin-allowlisted and therefore already trusted, so this is a least-privilege weakness rather than an external-attacker escalation, and an operator wanting strict separation can deploy one adapter per topic. No ONCHAINID key or role is escalated; the effect is that topic-level issuer separation is not enforced within one adapter.

Consider scoping attester authorization to the topic or the schema, for example keying the allowlist as _isAttesterAllowed[topic][attester] and taking the topic in setAttester, so that trusting an attester for one topic does not authorize it for the rest. If the global allowlist is intended, consider documenting that every allowlisted attester is trusted for every configured topic and recommending a separate adapter instance per topic.

Update: Resolved at commit d0b3db3 on PR66.

Expired pendingLinks Entries Are Undeletable and Accumulate Permanently

The factory stages inbound cross-chain link proposals in pendingLinks inside _processMessage and finalizes them through confirmCrossChainLink. That confirmation is the only path that deletes an entry, and it requires block.timestamp <= pending.expiry before reaching the delete, so once a proposal has expired the call reverts and the deletion is never performed. The interface exposes only a read-only getPendingCrossChainLink getter and no cancel or purge primitive, and removing a trusted gateway does not clear proposals it staged, so expired entries have no removal path at all.

A trusted gateway that is publicly usable, or a relayer that subsidizes delivery, can therefore stage many proposals under distinct wallet envelopes that are never confirmed before expiry, leaving each one in factory storage permanently. The result is unbounded, irreversible storage growth rather than a loss of funds or a change of authorization, which holds this at Low, and it is gated on a trusted gateway delivering the proposals.

Consider adding an explicit cleanup path for pendingLinks, such as an identity-authorized cancel for a proposal naming its own identity, or a permissionless purge that deletes an entry once block.timestamp > expiry, so expired proposals cannot accumulate without bound.

Update: Resolved at commit db01502 on PR67.

EAS Claim Validation Does Not Bind the Stored ClaimData

EASClaimIssuer._resolve accepts a Structs.ClaimData argument and deliberately ignores it, deciding validity only from the live EAS attestation: its UID, schema, allowed attester, revocation, expiry and recipient binding. ERC734Validator._addClaim nevertheless writes the caller-supplied issuance time, validity bound and payload into storage unchanged, so a party authorized to add a claim can pair a genuine EAS UID with a fabricated payload and validity window: isClaimValid keeps reporting the claim valid because the adapter never inspects the stored data, while the persisted ERC-735 record bears no relation to what the EAS attester signed. This is a genuine IClaimIssuer contract violation, since the adapter authenticates a claim whose stored data it does not bind.

The exposure is limited to off-chain consumers that both trust this adapter and decode the mirrored payload, which holds it at Low: no on-chain path consumes the fabricated fields, and the digest path that would read the stored data reverts for EAS-issued claims since the adapter implements no EIP-712 domain.

Consider defining the expected EAS schema encoding per topic and binding every security-relevant ClaimData field to the attestation data before returning a valid result; if an EAS schema cannot encode those fields, do not surface them as issuer-authenticated data through this adapter.

Update: Resolved at commit 83e17c6 on PR68.

upgradeBeacon Rebinds the Key Registry of Every Identity Without Validation or Delay

Every identity is a BeaconProxy over the single factory-owned beacon, and an identity's trust anchors are not held in proxy storage: _registryModule and _identityFactory are immutables of the implementation, read from whichever implementation the beacon currently names. A single upgradeBeacon call, constrained only to a non-zero address that has code, therefore rebinds the key registry of every identity ever deployed at once, with no per-identity opt-out and no storage write to observe. An implementation carrying a different registry module would hold no keys for existing accounts, leaving claims unresolvable and onlyManagerOrSelf answering as the new module decides, and the break would be partial, since installed modules keep pointing at the validator installed at initialization while key writes route through the immutable.

Nothing in the upgrade path establishes continuity between the outgoing and incoming implementations, the same absence of validation applies to initializeBeacon, and although the restricted modifier supports AccessManager execution delays, the deploy script never assigns the selector a dedicated role, so it falls through to ADMIN_ROLE, which the script grants with a zero execution delay.

Consider validating continuity in upgradeBeacon by requiring the candidate implementation to report the same registryModule and identityFactory as the outgoing one and to have its initializers disabled, and assigning the selector a dedicated AccessManager role with a non-zero execution delay so an upgrade is observable before it takes effect.

Update: Resolved at commit e9c9357 on PR69.

Utilities Proxy Accepts Non-Initializing Constructor Data

IdentityUtilitiesProxy is a pass-through wrapper whose constructor forwards the implementation and initialization data to ERC1967Proxy and adds no validation. The pinned @openzeppelin-contracts v5.7.0-rc.0 reverts with ERC1967ProxyUninitialized on empty data, so a proxy deployed with no init call is already closed upstream, but that guard is only a length check: any non-empty payload naming a real implementation selector is accepted, and since IdentityUtilities declares no fallback, a deployment carrying an unrelated view call such as getTopic completes while leaving the proxy's AccessControl storage untouched.

Because _disableInitializers in the implementation constructor only affects the implementation's own storage, initialize remains callable through such a proxy by anyone, and the first caller receives DEFAULT_ADMIN_ROLE, the sole gate on _authorizeUpgrade, so that caller can immediately repoint the proxy at arbitrary code that every consumer of getClaimsWithTopicInfo would then trust. The reference deployment passes proper initialization data, so this is a deployment-error footgun in a contract whose only purpose is to be the deployment entry point, rather than a live vulnerability on the scripted path.

Consider making the wrapper validate its own constructor argument, requiring the leading four bytes of the initialization data to equal IdentityUtilities.initialize.selector, so a deployment that does not initialize the proxy cannot be broadcast at all.

Update: Resolved at commit 4b85d27 on PR70.

Executors That Need a Self-Targeted Call Hold Authority Over Every Target

The RecoveryModule contract documents that its address must be registered with the MANAGEMENT purpose, since the account would otherwise reject the self-targeted addKeyWithData call it dispatches. That grant cannot be narrowed: the _isKeyAuthorizedToCallTarget function requires MANAGEMENT only for the identity itself and the factory and ACTION for every other target, while the _keyHasPurpose function lets MANAGEMENT satisfy both.

The module therefore clears every target the account can reach, with arbitrary calldata and value, including the key registry, the factory's wallet-binding calls and unrelated external contracts. Nothing downstream narrows it either, as executeRecovery dispatches whatever ERC-7579 execution the guardians signed. The ERC-734 purpose set cannot express an authority limited to adding a key, so any executor needing one self-targeted operation receives the same blanket permission by construction.

Consider allowing an executor's authority to be scoped to the operations it performs, through a dedicated purpose or a target and selector allowlist, so that a module requiring a single self-targeted call need not hold MANAGEMENT over everything else.

Notes & Additional Information

Documentation Improvement Suggestions

Several comments describe behavior that differs from the code.

The user-operation authorization model. The installModule NatSpec states the shipped model, namely that the account does not re-check a user operation the validator accepted, and the _authorizeCall function applies the per-target purpose rule only when the caller is an installed executor, which the EntryPoint never is. Four sites state the opposite:

• The SmartAccount header says "the per-target rule for user ops runs here", contradicting the @dev block twenty lines below it.
• The MockStockECDSAValidator contract states that "the ACTION / MANAGEMENT gate is enforced at the account layer against the validator address", which reads as an assurance to anyone installing a stock validator.
• The @notice above test_validateUserOp_installedStockValidator_noAccountPurpose_nowPasses says a validator not granted ACTION "cannot authorize a userOp against an external target", contradicting the test's own name, body, and the @dev block directly beneath it.
• The README's architecture section says the account "decides who is allowed to run transactions … using the key registry for authorization".

The ERC-7913 returndata length check. The _verify function reaches the verifier through a raw staticcall, so nothing is ABI-decoded and result.length >= 32 is the only constraint on returndata length. The comment calls that condition not strictly needed, on the grounds that a short result would cast to a zero-padded bytes32 and fail the comparison. Conversion from bytes to a fixed-size type pads on the right, so a verifier returning only the four bytes of the magic value produces exactly bytes32(IERC7913SignatureVerifier.verify.selector) and passes the comparison.

The ERC-7562 rationale for the signature design. The _verify function is documented as taking the ECDSA path directly to avoid the generic signature checker's signer.code.length check, which it says violates ERC-7562 bundler rules, and an inline comment claims the design keeps the ERC-4337 validation path free of an external call to an arbitrary signer. Its closing fall-through contradicts both: whenever ECDSA recovery does not return the expected signer it calls the ERC-1271 checker, a raw static call to the signer, unconditionally on the signer having code. Because _rawERC7579Validation reaches _verify and _validateUserOp routes a user operation through the same check, a user operation signed by a contract wallet makes exactly the external call the comment invokes ERC-7562 to avoid. Nothing is exploitable and the property holds for the EOA fast path, but the comment describes an invariant the code does not maintain for the contract signer the design was meant to support.

Consider revising each site to state what the code does: for the user-operation path, that the account performs no ERC-734 purpose check and the installed validator is trusted to scope its own signers, with the per-target rule in _authorizeCall applying only to executor callers; for _verify, that the length check is required because the raw staticcall bypasses ABI decoding, alongside documenting that verifiers are expected to ABI-encode their return value; and that the ERC-7562 rationale on _verify holds only for the EOA fast path, since ERC-1271 signers incur an external call during user-operation validation.

Update: Resolved at commit e4c3fe6 on PR71.

Immutable Beacon Address Assumes Canonical CREATE3 and Bricks the Factory on Non-Canonical Chains

The factory commits to its beacon at a predetermined address: beacon is an immutable set in the constructor to Create3.computeAddress(_BEACON_SALT), and initializeBeacon later deploys the UpgradeableBeacon there through Create3.deploy, discarding the returned address. On chains whose CREATE2 address derivation differs from the canonical EVM formula, Create3.computeAddress does not match where Create3.deploy actually deploys, so the beacon lands at an address the immutable does not name. Every identity deployment then reverts, because _deployIdentity requires beacon.code.length != 0 at the immutable address, and subsequent upgradeBeacon calls target the wrong address.

The repository already documents that CREATE3 determinism depends on canonical CREATE2 and that chains such as zkSync Era deviate (see README.md), so this is a portability limitation of the deployment rather than a defect on canonical EVM chains, where the factory works as intended.

Consider storing the beacon address returned by Create3.deploy in storage and asserting it matches Create3.computeAddress, reverting on mismatch to avoid partial initialization, or, if only canonical chains are targeted, enforcing that constraint explicitly and documenting it alongside the existing CREATE2 and CREATE3 caveats.

Update: Resolved at commit 12fa409 on PR72.

Gateway Allowlist Ignores the Origin Chain Encoded in the ERC-7930 Sender

The factory's ERC-7786 receive path stages cross-chain link proposals in _processMessage, authorizing the delivering gateway through _isAuthorizedGateway, which returns trustedGateways[gateway] and ignores the sender argument, an authenticated ERC-7930 interoperable address that encodes the origin chain. For a single gateway address that forwards messages from multiple origin chains, allowlisting that address implicitly accepts proposals from every origin it serves, so a per-origin policy such as accepting one chain while rejecting another cannot be expressed, and an origin-specific gateway misconfiguration has a wider blast radius.

The exposure is defense-in-depth rather than a path open to an unprivileged caller: a trusted gateway is already relied on for the whole wallet half of proof-of-control, and any binding still requires the named identity to confirm the proposal through confirmCrossChainLink with a MANAGEMENT key. An unintended origin can create or overwrite a pending proposal, but not finalize a binding on its own.

Consider extracting the origin identifiers from sender and scoping authorization by (gateway, chainType, chainReference), or explicitly documenting that trusting a gateway address implies trusting every origin chain it forwards.

Update: Resolved at commit 91d4586 on PR73.

Factory Notes: Admin-Role Sentinel Collision, Unbounded Getter, Unannotated Assembly, and Last-Wallet Revocation

This note collects four independent, low-impact observations on the identity factory.

• The TypePolicy struct uses a roleId of zero as the sentinel for an unregistered type, but in the pinned OpenZeppelin release the ADMIN_ROLE identifier is itself zero, so the one role that identifies the manager's own administrators cannot be stored in a policy. _checkTypeRole rejects the zero value before consulting the authority, so an administrator who calls setIdentityTypePolicy intending to restrict a type to administrators instead unregisters it for every caller, while the setter validates nothing and emits its event as though it had succeeded. No privilege is granted, and the workaround is a dedicated role identifier that the deploy script already uses, but the interface documentation does not record that the administrator role is unusable here.
• The single-argument getAccounts passes the full set length to _accountsRange, which copies every key and loads each stored ERC-7930 envelope, with neither the number of links nor the envelope size capped. Growth is self-inflicted, since only the identity links its own wallets, but once the set is large enough the accessor exceeds the gas cap providers apply to eth_call. A paginated overload sits beside it, yet the contract-level NatSpec still directs integrators to read the whole set, and the same shape recurs in IdentityUtilities.getClaimsWithTopicInfo.
• Most inline assembly blocks carry the memory-safe dialect annotation, but IdentityFactory._storage, ReputationRegistry._storage and KeyApprovalModule._msgSender do not, even though the last is a verbatim copy of the annotated ERC734Validator._msgSender. None touches memory, so correctness is unaffected; the omission only suppresses the memory-related optimizations the annotation unlocks and leaves identical code claiming two different things.
• revokeAccount drops a wallet from the identity's active set, so revoking an identity's only linked wallet leaves wallet-to-identity discovery resolving nothing and the identity appearing unreachable. That state is not permanent: wallet links and ERC-734 keys are separate namespaces, revocation never touches the keys, and the identity uses its unchanged MANAGEMENT key to call linkAccount again. The only durable consequence is that the specific revoked envelope cannot be re-linked.

Consider carrying registration in a dedicated boolean field of TypePolicy, or documenting that the administrator role cannot be a type's required role; replacing the unbounded getAccounts with the paginated form and updating the NatSpec; adding the memory-safe annotation to the three unannotated assembly blocks; and documenting that last-wallet revocation retires only the revoked address while the identity remains manageable and re-linkable, so integrators do not treat a momentarily empty set as a dead identity.

Update: Resolved at commit 04aff57 on PR74.

EAS Attestations Remain Valid at block.timestamp == expirationTime

EASClaimIssuer._resolve maps an attestation's live state into a claim status, and its expiration branch returns Expired only when attestation.expirationTime != 0 && block.timestamp > attestation.expirationTime. Because the comparison is strict, an attestation remains valid for the entire block whose timestamp equals expirationTime, one boundary later than an integration that reads expiration as taking effect at block.timestamp >= expirationTime would expect. The window is a single boundary block at the exact expiry timestamp, so the practical effect is limited to a narrow one-block slip-through in time-gated compliance flows.

Consider using an inclusive comparison, block.timestamp >= attestation.expirationTime, while preserving the expirationTime != 0 sentinel, so the boundary matches the common interpretation and no one-block validity extension occurs.

Update: Resolved at commit 09ff421 on PR75.

Version and EIP-712 Domain Strings Are Hard-Coded in Four Places

Every identity is a beacon proxy over one shared implementation, so the release strings are compiled into that implementation. accountId returns "trex.onchainid.identity.v3.0.0" and version returns "3.0.0", two independent literals spelling the same version twice, with nothing deriving one from the other. They change only through a manual edit in a new implementation promoted by upgradeBeacon, which validates nothing beyond a non-zero address, so an upgrade that omits the edit leaves every identity advertising a stale version and a partial edit lets the two drift apart. No on-chain logic reads either value, so the consequence falls on off-chain consumers and on determining from the chain which implementation an identity executes.

Two further version literals live in the EIP-712 domains, and unlike the display strings they are not inert. Identity's constructor declares ("OnchainID", "1") and the IdentityFactory constructor independently declares ("IdentityFactory", "1"). The claim layer stores no digest and rebuilds one on every read, since _getClaimDigest reconstructs the separator from the issuer's live domain and _addClaim persists a claim under whichever domain was in force. Raising the domain version alongside the display strings would change the separator of every identity at once, and every previously stored claim signature would begin resolving as invalid. These two literals are consensus parameters of every signature the deployment has produced, not release markers, which is what makes a naive single-source-of-truth version scheme unsafe.

Consider deriving the account identifier and the version getter from a single internal constant so the version is written once, and asserting at upgrade time that the promoted implementation reports the expected value, while explicitly excluding the two EIP-712 domain versions from that scheme and documenting that the domain version is a consensus parameter of every claim signature the deployment has produced.

Update: Resolved at commit 59b1ea9 on PR76.

Thank you for the fix on this PR!

We have one minor, non-blocking documentation recommendation. The current comment says that the check rejects a build whose version was not bumped. Since expectedVersion is supplied by the upgrader, the check cannot detect a forgotten bump by itself. For example, if the candidate still reports 3.0.0 and the upgrader also passes 3.0.0, the upgrade succeeds even if the intended release was 4.0.0.

The check guarantees agreement between the candidate implementation and the version declared by the upgrader; it does not require the version to change.

We suggest wording it as:

The upgrade is rejected when the candidate implementation’s version differs from the version declared by the upgrader.

Or, for the inline code comment:

The version string is compiled into the implementation. Requiring it to match expectedVersion ensures that the upgrade calldata explicitly commits to the version reported by the candidate.

This does not affect the correctness of the implemented fix. We consider N-06 resolved, with this minor wording recommendation.

createIdentityFor Does Not Honour AccessManager Execution Delays

createIdentityFor is the factory's permissioned deploy path, and it gates on _checkTypeRole rather than the restricted modifier. That hand-rolled gate queries the AccessManager for role membership and keeps only the boolean, discarding the execution delay returned alongside it. A restricted function instead reaches the manager through canCall, which surfaces that delay and forces a holder configured with one to schedule the operation and wait, so the delay is silently inert on this entry point. A role granted with a non-zero execution delay, intended to time-lock the operator, can therefore call createIdentityFor and mint an identity together with its permanent wallet binding in the same block the role was granted.

The impact is bounded, which is why this is a Note: the caller already holds the role and is authorized to deploy the type, so only the timing constraint the administrator configured is bypassed, not the authorization itself. A per-type policy also cannot simply move onto the restricted modifier, because the AccessManager stores a single role per target and selector pair while createIdentityFor is one selector, so honouring the returned delay is the appropriate remedy rather than re-routing the gate.

Consider having _checkTypeRole read and enforce the execution delay the membership query already returns, either by rejecting an immediate call when the delay is non-zero or by integrating the AccessManager scheduling flow keyed to the specific identity type, so the per-type gate preserves the time-lock the operator was configured with.

Update: Resolved at commit f065b68 on PR77.

Thank you for the fix.

One operational detail should be made explicit: this fix does not implement AccessManager’s scheduling flow. A member with a delayed role cannot simply wait for the delay and then call createIdentityFor; the membership remains unsupported until its execution delay is changed to zero.

We therefore have one minor, non-blocking documentation recommendation: please document this constraint in the public IIdentityFactory.setIdentityTypePolicy NatSpec or the operator documentation, for example:

Per-type roles used by createIdentityFor must be granted with a zero execution delay. Memberships carrying a non-zero execution delay are unsupported and rejected.

Conclusion

ONCHAINID provides the on-chain identity layer for the T-REX (ERC-3643) permissioned-token framework, where a wallet's identity and its verified claims determine whether it is eligible to hold a regulated asset. The audited revision reimplements the identity as a modular ERC-7579 smart account, adding ERC-4337 account abstraction and support for secp256r1, RSA, and WebAuthn signers through ERC-7913. OpenZeppelin audited the identity account, the merged key and claim registry, the factory that binds wallets to identities, and the supporting claim, reputation, and recovery integrations.

The revision merges two authorization models, the ERC-734 key-purpose system and the ERC-7579 execution path, into a single account, and most of the identified risk originates in that merged logic and in the factory that records wallet-to-identity bindings. The most severe findings sit on this seam: a critical issue in batch-call decoding let an action key escape its calldata slice and seize management authority, and the high-severity issues each let an action key cross the same boundary through the enshrined key registry and the approval queue. A recurring pattern is that the components that authorize a call, namely the user-operation validator, the account's execution funnel, and the approval-queue module, do not always agree on what a call means or on which targets require management authority. A related pattern is that several security properties depend on how an identity is assembled at deployment rather than on invariants the contracts enforce, so a single configuration choice can remove a control. A third concerns the wallet-to-identity bindings that govern token eligibility, which are not fully validated on-chain and cannot be reversed once set. A fourth concerns the claim lifecycle: claim digests are recomputed from live issuer state, and adapter-backed attestations lack a revocation path, so a claim can become unverifiable or unremovable after issuance.

The codebase is thoroughly documented: the NatSpec states intended authority boundaries and design rationale precisely, which materially aided the audit. The test suite, by contrast, exercises expected behavior but does not cover the adversarial cases on the authorization boundaries, the binding registry, and the claim lifecycle where the findings arise, so expanding it in these areas is advisable. Enforcing a canonical module configuration in the contracts, rather than relying on deployment conventions, would remove a class of the identified exposures. The social-recovery module's upstream logic was outside the reviewed scope and merits a dedicated review.

OpenZeppelin thanks the T-REX team for their responsiveness and collaboration throughout the engagement.

Appendix

Issue Classification

OpenZeppelin classifies smart contract vulnerabilities on a 5-level scale:

• Critical
• High
• Medium
• Low
• Note/Information

Critical Severity

This classification is applied when the issue’s impact is catastrophic, threatening extensive damage to the client's reputation and/or causing severe financial loss to the client or users. The likelihood of exploitation can be high, warranting a swift response. Critical issues typically involve significant risks such as the permanent loss or locking of a large volume of users' sensitive assets or the failure of core system functionalities without viable mitigations. These issues demand immediate attention due to their potential to compromise system integrity or user trust significantly.

High Severity

These issues are characterized by the potential to substantially impact the client’s reputation and/or result in considerable financial losses. The likelihood of exploitation is significant, warranting a swift response. Such issues might include temporary loss or locking of a significant number of users' sensitive assets or disruptions to critical system functionalities, albeit with potential, yet limited, mitigations available. The emphasis is on the significant but not always catastrophic effects on system operation or asset security, necessitating prompt and effective remediation.

Medium Severity

Issues classified as being of medium severity can lead to a noticeable negative impact on the client's reputation and/or moderate financial losses. Such issues, if left unattended, have a moderate likelihood of being exploited or may cause unwanted side effects in the system. These issues are typically confined to a smaller subset of users' sensitive assets or might involve deviations from the specified system design that, while not directly financial in nature, compromise system integrity or user experience. The focus here is on issues that pose a real but contained risk, warranting timely attention to prevent escalation.

Low Severity

Low-severity issues are those that have a low impact on the client's operations and/or reputation. These issues may represent minor risks or inefficiencies to the client's specific business model. They are identified as areas for improvement that, while not urgent, could enhance the security and quality of the codebase if addressed.

Notes & Additional Information Severity

This category is reserved for issues that, despite having a minimal impact, are still important to resolve. Addressing these issues contributes to the overall security posture and code quality improvement but does not require immediate action. It reflects a commitment to maintaining high standards and continuous improvement, even in areas that do not pose immediate risks.