From dea806cfee1ef754dc5a134ef98a745d5096db08 Mon Sep 17 00:00:00 2001 From: Pawan Upadhyay <141557022+pawansatoshi@users.noreply.github.com> Date: Thu, 10 Sep 2026 17:57:04 +0530 Subject: [PATCH 1/3] fix(genesis): normalize hex identity comparisons --- .../genesis-identity-normalization.md | 57 ++++++++++++ scripts/genesis/NativeFiatToken.ts | 7 +- scripts/genesis/ValidatorManager.ts | 32 ++++--- scripts/genesis/types.ts | 3 +- ...tive-fiat-token-genesis-validation.test.ts | 71 +++++++++++++++ ...lidator-manager-genesis-validation.test.ts | 89 ++++++++++++++++++- 6 files changed, 242 insertions(+), 17 deletions(-) create mode 100644 docs/security/genesis-identity-normalization.md create mode 100644 tests/unit/native-fiat-token-genesis-validation.test.ts diff --git a/docs/security/genesis-identity-normalization.md b/docs/security/genesis-identity-normalization.md new file mode 100644 index 00000000..9249e4e8 --- /dev/null +++ b/docs/security/genesis-identity-normalization.md @@ -0,0 +1,57 @@ +# Genesis identity normalization + +## Finding + +`schemaAddress` and `schemaHex` accept mixed-case hexadecimal identities, while several genesis validation checks historically compared the textual representation directly. Hexadecimal casing does not change the represented bytes, so two spellings of the same address or fixed-size public key can bypass string-based uniqueness or role-separation checks. + +## Affected validation paths + +- Validator public-key uniqueness +- Validator-registerer uniqueness +- Controller uniqueness across validators +- Operator/role versus proxy-admin separation +- ValidatorRegistry proxy-address comparison +- NativeFiatToken minter uniqueness + +## Impact + +This is a genesis/configuration integrity issue rather than a standalone permissionless runtime exploit. A malformed configuration could represent the same underlying identity more than once where validation intends uniqueness or role separation. + +For NativeFiatToken minters, address-keyed storage is derived from the decoded address value. Case variants therefore resolve to the same mapping slots; conflicting allowance entries can overwrite one another during genesis allocation construction instead of being rejected as duplicate minters. + +## Fix + +Normalize hexadecimal identities only at comparison boundaries with `toLowerCase()`. Keep the original input unchanged for serialization and diagnostics. The shared proxy-admin helper normalizes both operands, and NativeFiatToken minter uniqueness normalizes the address used as the `Set` key. + +## Regression coverage + +Regression tests cover mixed-case collisions for: + +- Validator public keys +- Validator-registerer addresses +- Controllers +- Operator/proxy-admin role separation +- NativeFiatToken minters + +The tests also retain positive cases for valid distinct/compatible configurations. + +## Validation + +Focused tests: + +```bash +npx hardhat test ./tests/unit/validator-manager-genesis-validation.test.ts ./tests/unit/native-fiat-token-genesis-validation.test.ts --no-compile +``` + +Repository validation: + +```bash +make test-unit-hardhat +make lint +``` + +The repository workflow does not currently invoke `make test-unit-hardhat`, and its ESLint command excludes `scripts/` and `tests/`, so local execution of the focused suite remains an explicit validation requirement. + +## Scope + +EIP-55 checksum enforcement is intentionally not part of this change. It would be a broader input-validation policy change and should be reviewed independently. diff --git a/scripts/genesis/NativeFiatToken.ts b/scripts/genesis/NativeFiatToken.ts index ba64d2a6..46e8be76 100644 --- a/scripts/genesis/NativeFiatToken.ts +++ b/scripts/genesis/NativeFiatToken.ts @@ -78,15 +78,16 @@ export const schemaNativeFiatToken = z })), ]) - const minterSet = new Set() + const minterSet = new Set() for (const minter of data.minters) { - if (minterSet.has(minter.address)) { + const normalized = minter.address.toLowerCase() + if (minterSet.has(normalized)) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: `Minter ${minter.address} must be unique`, }) } - minterSet.add(minter.address) + minterSet.add(normalized) } }) diff --git a/scripts/genesis/ValidatorManager.ts b/scripts/genesis/ValidatorManager.ts index 713546cd..8ad573a7 100644 --- a/scripts/genesis/ValidatorManager.ts +++ b/scripts/genesis/ValidatorManager.ts @@ -107,16 +107,18 @@ export const schemaValidatorManager = z message: 'At least one validator must have positive voting power', }) } - // Verify the public keys are unique. - const publicKeySet = new Set() + + // Verify the public keys are unique by their byte identity, not hex casing. + const publicKeySet = new Set() for (const validator of data.validators) { - if (publicKeySet.has(validator.publicKey)) { + const normalizedPublicKey = validator.publicKey.toLowerCase() + if (publicKeySet.has(normalizedPublicKey)) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: `Public key ${validator.publicKey} must be unique`, }) } - publicKeySet.add(validator.publicKey) + publicKeySet.add(normalizedPublicKey) } const permissionedManager = data.PermissionedValidatorManager @@ -138,29 +140,35 @@ export const schemaValidatorManager = z ...flattenedControllers.map(({ key, address }) => ({ key, value: address })), ]) - // Verify addresses are unique for different roles. - const validatorRegistererSet = new Set() + // Verify addresses are unique for different roles by their byte identity. + const validatorRegistererSet = new Set() for (const validatorRegisterer of permissionedManager.validatorRegisterers) { - if (validatorRegistererSet.has(validatorRegisterer)) { + const normalizedValidatorRegisterer = validatorRegisterer.toLowerCase() + if (validatorRegistererSet.has(normalizedValidatorRegisterer)) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: `ValidatorRegisterer ${validatorRegisterer} must be unique`, }) } - validatorRegistererSet.add(validatorRegisterer) + validatorRegistererSet.add(normalizedValidatorRegisterer) } - const controllerSet = new Set
() + + const controllerSet = new Set() for (const { address, key } of flattenedControllers) { - if (controllerSet.has(address)) { + const normalizedAddress = address.toLowerCase() + if (controllerSet.has(normalizedAddress)) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: `Controller ${address} (${key}) must be unique across all validators`, }) } - controllerSet.add(address) + controllerSet.add(normalizedAddress) } - if (data.proxy.address != null && data.proxy.address !== DEFAULT_VALIDATOR_REGISTRY_PROXY_ADDRESS) { + if ( + data.proxy.address != null && + data.proxy.address.toLowerCase() !== DEFAULT_VALIDATOR_REGISTRY_PROXY_ADDRESS.toLowerCase() + ) { // the ValidatorRegistry address is hardcoded in the PermissionedValidatorManager. ctx.addIssue({ code: z.ZodIssueCode.custom, diff --git a/scripts/genesis/types.ts b/scripts/genesis/types.ts index 5181f47d..8fa4f6ef 100644 --- a/scripts/genesis/types.ts +++ b/scripts/genesis/types.ts @@ -159,8 +159,9 @@ export const enforceOperatorsNotProxyAdmin = ( proxyAdmin: Address, operators: ReadonlyArray<{ key: string; value: Address }>, ) => { + const normalizedProxyAdmin = proxyAdmin.toLowerCase() for (const { key, value } of operators) { - if (value === proxyAdmin) { + if (value.toLowerCase() === normalizedProxyAdmin) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: `Operator ${key} cannot be the same as the proxy admin of ${contractName}`, diff --git a/tests/unit/native-fiat-token-genesis-validation.test.ts b/tests/unit/native-fiat-token-genesis-validation.test.ts new file mode 100644 index 00000000..04e9a642 --- /dev/null +++ b/tests/unit/native-fiat-token-genesis-validation.test.ts @@ -0,0 +1,71 @@ +// Copyright 2026 Circle Internet Group, Inc. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 + +import { expect } from 'chai' +import { schemaNativeFiatToken } from '../../scripts/genesis/NativeFiatToken' + +const PROXY_ADMIN = '0x1111111111111111111111111111111111111111' +const OWNER = '0x2222222222222222222222222222222222222222' +const PAUSER = '0x3333333333333333333333333333333333333333' +const BLACKLISTER = '0x4444444444444444444444444444444444444444' +const MASTER_MINTER = '0x5555555555555555555555555555555555555555' +const RESCUER = '0x6666666666666666666666666666666666666666' + +const configWithMinters = (minters: Array<{ address: string; allowance: bigint }>) => ({ + proxy: { admin: PROXY_ADMIN }, + owner: OWNER, + pauser: PAUSER, + blacklister: BLACKLISTER, + masterMinter: MASTER_MINTER, + rescuer: RESCUER, + minters, +}) + +describe('NativeFiatToken genesis validation', () => { + it('rejects duplicate minters with identical casing', () => { + const minter = '0xAb00000000000000000000000000000000000001' + const result = schemaNativeFiatToken.safeParse( + configWithMinters([ + { address: minter, allowance: 100n }, + { address: minter, allowance: 200n }, + ]), + ) + + expect(result.success).to.be.false + }) + + it('rejects duplicate minters when address casing differs', () => { + const minter = '0xAb00000000000000000000000000000000000001' + const result = schemaNativeFiatToken.safeParse( + configWithMinters([ + { address: minter, allowance: 100n }, + { address: minter.toLowerCase(), allowance: 200n }, + ]), + ) + + expect(result.success).to.be.false + }) + + it('accepts distinct minter addresses', () => { + const result = schemaNativeFiatToken.safeParse( + configWithMinters([ + { address: '0xAb00000000000000000000000000000000000001', allowance: 100n }, + { address: '0xAb00000000000000000000000000000000000002', allowance: 200n }, + ]), + ) + + expect(result.success).to.be.true + }) + + it('rejects a role colliding with the proxy admin when address casing differs', () => { + const proxyAdmin = '0xAb00000000000000000000000000000000000003' + const result = schemaNativeFiatToken.safeParse({ + ...configWithMinters([]), + proxy: { admin: proxyAdmin }, + owner: proxyAdmin.toLowerCase(), + }) + + expect(result.success).to.be.false + }) +}) diff --git a/tests/unit/validator-manager-genesis-validation.test.ts b/tests/unit/validator-manager-genesis-validation.test.ts index 4ab6d5fd..c2ffb6ef 100644 --- a/tests/unit/validator-manager-genesis-validation.test.ts +++ b/tests/unit/validator-manager-genesis-validation.test.ts @@ -2,6 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 +import { z } from 'zod' import { expect } from 'chai' import { schemaValidatorManager } from '../../scripts/genesis/ValidatorManager' @@ -16,6 +17,8 @@ const CONTROLLER_B = '0x7777777777777777777777777777777777777777' const PUBLIC_KEY_A = `0x${'11'.repeat(32)}` const PUBLIC_KEY_B = `0x${'22'.repeat(32)}` +type ValidatorManagerConfig = z.infer + const validator = (publicKey: string, controller: string, votingPower: bigint) => ({ publicKey, votingPower, @@ -27,7 +30,10 @@ const validator = (publicKey: string, controller: string, votingPower: bigint) = ], }) -const configWithValidators = (validators: ReturnType[]) => ({ +const configWithValidators = ( + validators: ReturnType[], + overrides: Partial = {}, +) => ({ proxy: { admin: REGISTRY_ADMIN, }, @@ -40,6 +46,7 @@ const configWithValidators = (validators: ReturnType[]) => ({ pauser: PAUSER, validatorRegisterers: [REGISTERER], }, + ...overrides, }) describe('ValidatorManager genesis validator-set validation', () => { @@ -70,4 +77,84 @@ describe('ValidatorManager genesis validator-set validation', () => { expect(result.success).to.be.true }) + + it('rejects duplicate public keys when hexadecimal casing differs', () => { + const publicKey = `0x${'Ab'.repeat(32)}` + const result = schemaValidatorManager.safeParse( + configWithValidators([validator(publicKey, CONTROLLER_A, 20n), validator(publicKey.toLowerCase(), CONTROLLER_B, 10n)]), + ) + + expect(result.success).to.be.false + }) + + it('rejects duplicate controllers when address casing differs', () => { + const controller = '0xAb00000000000000000000000000000000000001' + const result = schemaValidatorManager.safeParse( + configWithValidators([validator(PUBLIC_KEY_A, controller, 20n), validator(PUBLIC_KEY_B, controller.toLowerCase(), 10n)]), + ) + + expect(result.success).to.be.false + }) + + it('rejects duplicate validator registerers when address casing differs', () => { + const registerer = '0xAb00000000000000000000000000000000000002' + const result = schemaValidatorManager.safeParse( + configWithValidators([validator(PUBLIC_KEY_A, CONTROLLER_A, 20n)], { + PermissionedValidatorManager: { + proxy: { admin: PVM_ADMIN }, + owner: OWNER, + pauser: PAUSER, + validatorRegisterers: [registerer, registerer.toLowerCase()], + }, + }), + ) + + expect(result.success).to.be.false + }) + + it('rejects an operator colliding with the proxy admin when address casing differs', () => { + const proxyAdmin = '0xAb00000000000000000000000000000000000003' + const result = schemaValidatorManager.safeParse( + configWithValidators([validator(PUBLIC_KEY_A, CONTROLLER_A, 20n)], { + PermissionedValidatorManager: { + proxy: { admin: proxyAdmin }, + owner: proxyAdmin.toLowerCase(), + pauser: PAUSER, + validatorRegisterers: [REGISTERER], + }, + }), + ) + + expect(result.success).to.be.false + }) + + it('rejects a controller colliding with the PVM proxy admin when address casing differs', () => { + const proxyAdmin = '0xAb00000000000000000000000000000000000004' + const result = schemaValidatorManager.safeParse( + configWithValidators([validator(PUBLIC_KEY_A, proxyAdmin.toLowerCase(), 20n)], { + PermissionedValidatorManager: { + proxy: { admin: proxyAdmin }, + owner: OWNER, + pauser: PAUSER, + validatorRegisterers: [REGISTERER], + }, + }), + ) + + expect(result.success).to.be.false + }) + + it('accepts a ValidatorRegistry proxy address when only hexadecimal casing differs', () => { + const proxyAddress = '0x3600000000000000000000000000000000000002' + const result = schemaValidatorManager.safeParse( + configWithValidators([validator(PUBLIC_KEY_A, CONTROLLER_A, 20n)], { + proxy: { + address: proxyAddress.toUpperCase().replace('0X', '0x'), + admin: REGISTRY_ADMIN, + }, + }), + ) + + expect(result.success).to.be.true + }) }) From 3b19c887dd6c2ccd29f50be1e923593aeb1240e4 Mon Sep 17 00:00:00 2001 From: Pawan Upadhyay <141557022+pawansatoshi@users.noreply.github.com> Date: Fri, 11 Sep 2026 06:32:12 +0530 Subject: [PATCH 2/3] test(genesis): remove vacuous proxy casing case --- .../validator-manager-genesis-validation.test.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/unit/validator-manager-genesis-validation.test.ts b/tests/unit/validator-manager-genesis-validation.test.ts index c2ffb6ef..c306788b 100644 --- a/tests/unit/validator-manager-genesis-validation.test.ts +++ b/tests/unit/validator-manager-genesis-validation.test.ts @@ -143,18 +143,4 @@ describe('ValidatorManager genesis validator-set validation', () => { expect(result.success).to.be.false }) - - it('accepts a ValidatorRegistry proxy address when only hexadecimal casing differs', () => { - const proxyAddress = '0x3600000000000000000000000000000000000002' - const result = schemaValidatorManager.safeParse( - configWithValidators([validator(PUBLIC_KEY_A, CONTROLLER_A, 20n)], { - proxy: { - address: proxyAddress.toUpperCase().replace('0X', '0x'), - admin: REGISTRY_ADMIN, - }, - }), - ) - - expect(result.success).to.be.true - }) }) From 27d3f99f480ac68f0b7e9f1117bcf5269b7c1acb Mon Sep 17 00:00:00 2001 From: Pawan Upadhyay <141557022+pawansatoshi@users.noreply.github.com> Date: Fri, 11 Sep 2026 06:32:18 +0530 Subject: [PATCH 3/3] docs(security): clarify affected genesis paths --- docs/security/genesis-identity-normalization.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/security/genesis-identity-normalization.md b/docs/security/genesis-identity-normalization.md index 9249e4e8..8da5f233 100644 --- a/docs/security/genesis-identity-normalization.md +++ b/docs/security/genesis-identity-normalization.md @@ -10,9 +10,10 @@ - Validator-registerer uniqueness - Controller uniqueness across validators - Operator/role versus proxy-admin separation -- ValidatorRegistry proxy-address comparison - NativeFiatToken minter uniqueness +The ValidatorRegistry proxy-address comparison is not an affected casing path: the current hard-coded system-contract address contains only numeric hexadecimal digits, so no alternate mixed-case spelling can exist. + ## Impact This is a genesis/configuration integrity issue rather than a standalone permissionless runtime exploit. A malformed configuration could represent the same underlying identity more than once where validation intends uniqueness or role separation.