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)) 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'; } /**