From 4fa395556aa6679e5adf43117b316422252a9b2f Mon Sep 17 00:00:00 2001 From: ZayanKhan-12 <108294002+ZayanKhan-12@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:04:35 -0400 Subject: [PATCH 1/2] fix: widen `isSafeDynamicKey` parameter type to `PropertyKey` `isSafeDynamicKey` only accepted `string` keys and returned `false` for any non-string input, so valid dynamic keys of type `number` (e.g. array indices such as `currentIndex` in SmartTransactionsController) or `symbol` could not be validated without a lossy `String()` conversion. Number and symbol keys cannot be used for prototype pollution via the blocklisted string keys: numbers coerce to numeric strings that never collide with the blocklist, and symbol keys cannot alias string-keyed `Object.prototype` properties. The function now accepts any `PropertyKey`, keeps the blocklist check for strings, and still returns `false` for non-`PropertyKey` runtime values passed from untyped code. Fixes MetaMask/utils#200 Co-Authored-By: Claude Fable 5 --- packages/controller-utils/src/util.test.ts | 9 ++++++++- packages/controller-utils/src/util.ts | 17 ++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/controller-utils/src/util.test.ts b/packages/controller-utils/src/util.test.ts index e3346ea473f..19f9e61d47b 100644 --- a/packages/controller-utils/src/util.test.ts +++ b/packages/controller-utils/src/util.test.ts @@ -33,8 +33,15 @@ describe('util', () => { for (const badKey of util.PROTOTYPE_POLLUTION_BLOCKLIST) { expect(util.isSafeDynamicKey(badKey)).toBe(false); } - // @ts-expect-error - ensure that non-string input return false. + expect(util.isSafeDynamicKey(0)).toBe(true); + expect(util.isSafeDynamicKey(123)).toBe(true); + expect(util.isSafeDynamicKey(Number.NaN)).toBe(true); + expect(util.isSafeDynamicKey(Symbol('__proto__'))).toBe(true); + expect(util.isSafeDynamicKey(Symbol.iterator)).toBe(true); + // @ts-expect-error - ensure that non-`PropertyKey` input returns false. expect(util.isSafeDynamicKey(null)).toBe(false); + // @ts-expect-error - ensure that non-`PropertyKey` input returns false. + expect(util.isSafeDynamicKey(undefined)).toBe(false); }); it('isSafeChainId', () => { expect(util.isSafeChainId(util.toHex(MAX_SAFE_CHAIN_ID + 1))).toBe(false); diff --git a/packages/controller-utils/src/util.ts b/packages/controller-utils/src/util.ts index 71a0f8c5430..8d1790bdd14 100644 --- a/packages/controller-utils/src/util.ts +++ b/packages/controller-utils/src/util.ts @@ -31,14 +31,21 @@ export const PROTOTYPE_POLLUTION_BLOCKLIST = [ * Checks whether a dynamic property key could be used in * a [prototype pollution attack](https://portswigger.net/web-security/prototype-pollution). * + * String keys are compared against the {@link PROTOTYPE_POLLUTION_BLOCKLIST}. + * Number and symbol keys are always considered safe: number keys are coerced + * to numeric strings, which can never collide with the blocklist, and symbol + * keys cannot alias the string-keyed `Object.prototype` properties. + * * @param key - The dynamic key to validate. * @returns Whether the given dynamic key is safe to use. */ -export function isSafeDynamicKey(key: string): boolean { - return ( - typeof key === 'string' && - !PROTOTYPE_POLLUTION_BLOCKLIST.some((blockedKey) => key === blockedKey) - ); +export function isSafeDynamicKey(key: PropertyKey): boolean { + if (typeof key === 'string') { + return !PROTOTYPE_POLLUTION_BLOCKLIST.some( + (blockedKey) => key === blockedKey, + ); + } + return typeof key === 'number' || typeof key === 'symbol'; } /** From 2034caab9f87cade29897bdbaa2366b0d14297f5 Mon Sep 17 00:00:00 2001 From: ZayanKhan-12 <108294002+ZayanKhan-12@users.noreply.github.com> Date: Mon, 3 Aug 2026 21:05:20 -0400 Subject: [PATCH 2/2] docs: add changelog entry Co-Authored-By: Claude Fable 5 --- packages/controller-utils/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index 487e484d820..19d247d5f78 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/packages/controller-utils/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add optional `startTime` to `TraceRequest` to allow backdating a span's start time ([#9315](https://github.com/MetaMask/core/pull/9315)) +### Changed + +- Widen `isSafeDynamicKey` parameter type from `string` to `PropertyKey` ([#9774](https://github.com/MetaMask/core/pull/9774)) + - `number` and `symbol` keys are now considered safe and return `true`; previously any non-string input returned `false` + ### Deprecated - Deprecate `createServicePolicy` and related symbols ([#9418](https://github.com/MetaMask/core/pull/9418))