diff --git a/docs/security/genesis-identity-normalization.md b/docs/security/genesis-identity-normalization.md new file mode 100644 index 0000000..9249e4e --- /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 ba64d2a..46e8be7 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 713546c..8ad573a 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 5181f47..8fa4f6e 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 0000000..04e9a64 --- /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 4ab6d5f..c2ffb6e 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 + }) })