From 03712f0baa924afd2b519bda06294dc9e7dffe72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rmungandrk?= <47859767+odaysec@users.noreply.github.com> Date: Fri, 19 Sep 2025 04:17:50 +0700 Subject: [PATCH] fix(packages): Prevent prototype pollution by Validating dynamic keys across controllers --- .../src/AccountTrackerController.ts | 16 ++++++++++++++++ .../src/TokenBalancesController.ts | 16 ++++++++++++++++ packages/earn-controller/src/selectors.ts | 14 +++++++++----- packages/ens-controller/src/EnsController.ts | 5 ++++- packages/name-controller/src/NameController.ts | 6 +++--- .../src/NetworkEnablementController.ts | 11 +++++++++++ .../src/sample-petnames-controller.ts | 8 ++++++++ 7 files changed, 67 insertions(+), 9 deletions(-) diff --git a/packages/assets-controllers/src/AccountTrackerController.ts b/packages/assets-controllers/src/AccountTrackerController.ts index f8b3841908e..68ee59918b6 100644 --- a/packages/assets-controllers/src/AccountTrackerController.ts +++ b/packages/assets-controllers/src/AccountTrackerController.ts @@ -704,6 +704,14 @@ export class AccountTrackerController extends StaticIntervalPollingController { balances.forEach(({ address, chainId, balance }) => { + // Prevent prototype pollution with dangerous keys + if ( + chainId === '__proto__' || + chainId === 'constructor' || + chainId === 'prototype' + ) { + return; + } const checksumAddress = toChecksumHexAddress(address); // Ensure the chainId exists in the state @@ -740,6 +748,14 @@ export class AccountTrackerController extends StaticIntervalPollingController { stakedBalances.forEach(({ address, chainId, stakedBalance }) => { + // Prevent prototype pollution with dangerous keys + if ( + chainId === '__proto__' || + chainId === 'constructor' || + chainId === 'prototype' + ) { + return; + } const checksumAddress = toChecksumHexAddress(address); // Ensure the chainId exists in the state diff --git a/packages/assets-controllers/src/TokenBalancesController.ts b/packages/assets-controllers/src/TokenBalancesController.ts index c4d21b35658..bf1cdde555b 100644 --- a/packages/assets-controllers/src/TokenBalancesController.ts +++ b/packages/assets-controllers/src/TokenBalancesController.ts @@ -594,6 +594,14 @@ export class TokenBalancesController extends StaticIntervalPollingController<{ // First, initialize all tokens from allTokens state with balance 0 // for the accounts and chains we're processing for (const chainId of targetChains) { + // Prevent prototype pollution by skipping dangerous property names + if ( + chainId === '__proto__' || + chainId === 'constructor' || + chainId === 'prototype' + ) { + continue; + } for (const account of accountsToProcess) { // Initialize tokens from allTokens const chainTokens = this.#allTokens[chainId]; @@ -625,6 +633,14 @@ export class TokenBalancesController extends StaticIntervalPollingController<{ // Then update with actual fetched balances where available aggregated.forEach(({ success, value, account, token, chainId }) => { + // Prevent prototype pollution by skipping dangerous property names + if ( + chainId === '__proto__' || + chainId === 'constructor' || + chainId === 'prototype' + ) { + return; + } if (success && value !== undefined) { ((d.tokenBalances[account] ??= {})[chainId] ??= {})[checksum(token)] = toHex(value); diff --git a/packages/earn-controller/src/selectors.ts b/packages/earn-controller/src/selectors.ts index af725541990..77b738cd21c 100644 --- a/packages/earn-controller/src/selectors.ts +++ b/packages/earn-controller/src/selectors.ts @@ -23,11 +23,13 @@ export const selectLendingMarketsByProtocolAndId = createSelector( (markets) => { return markets.reduce( (acc, market) => { - acc[market.protocol] = acc[market.protocol] || {}; - acc[market.protocol][market.id] = market; + if (!acc.has(market.protocol)) { + acc.set(market.protocol, new Map()); + } + acc.get(market.protocol)!.set(market.id, market); return acc; }, - {} as Record>, + new Map>(), ); }, ); @@ -38,7 +40,9 @@ export const selectLendingMarketForProtocolAndId = ( ) => createSelector( selectLendingMarketsByProtocolAndId, - (marketsByProtocolAndId) => marketsByProtocolAndId?.[protocol]?.[id], + (marketsByProtocolAndId) => { + return marketsByProtocolAndId?.get(protocol)?.get(id); + }, ); export const selectLendingMarketsByChainId = createSelector( @@ -63,7 +67,7 @@ export const selectLendingPositionsWithMarket = createSelector( return { ...position, market: - marketsByProtocolAndId?.[position.protocol]?.[position.marketId], + marketsByProtocolAndId?.get(position.protocol)?.get(position.marketId), }; }); }, diff --git a/packages/ens-controller/src/EnsController.ts b/packages/ens-controller/src/EnsController.ts index 14023726482..7caaa95ce3e 100644 --- a/packages/ens-controller/src/EnsController.ts +++ b/packages/ens-controller/src/EnsController.ts @@ -196,11 +196,14 @@ export class EnsController extends BaseController< */ delete(chainId: Hex, ensName: string): boolean { const normalizedEnsName = normalizeEnsName(ensName); + // Defense-in-depth: block prototype-polluting property names. + const unsafeKeys = ['__proto__', 'prototype', 'constructor']; if ( !isSafeDynamicKey(chainId) || !normalizedEnsName || !this.state.ensEntries[chainId] || - !this.state.ensEntries[chainId][normalizedEnsName] + !this.state.ensEntries[chainId][normalizedEnsName] || + unsafeKeys.includes(chainId) ) { return false; } diff --git a/packages/name-controller/src/NameController.ts b/packages/name-controller/src/NameController.ts index b37327916e6..a8779eb1181 100644 --- a/packages/name-controller/src/NameController.ts +++ b/packages/name-controller/src/NameController.ts @@ -467,14 +467,14 @@ export class NameController extends BaseController< } this.update((state) => { - const typeEntries = state.names[type] || {}; + const typeEntries = state.names[type] || Object.create(null); state.names[type] = typeEntries; - const variationEntries = typeEntries[normalizedValue] || {}; + const variationEntries = typeEntries[normalizedValue] || Object.create(null); typeEntries[normalizedValue] = variationEntries; const entry = variationEntries[normalizedVariation] ?? { - proposedNames: {}, + proposedNames: Object.create(null), name: null, sourceId: null, origin: null, diff --git a/packages/network-enablement-controller/src/NetworkEnablementController.ts b/packages/network-enablement-controller/src/NetworkEnablementController.ts index 856f0884d34..df5c5261047 100644 --- a/packages/network-enablement-controller/src/NetworkEnablementController.ts +++ b/packages/network-enablement-controller/src/NetworkEnablementController.ts @@ -252,6 +252,17 @@ export class NetworkEnablementController extends BaseController< ); } + // Prevent prototype pollution via dangerous property names + if ( + namespace === '__proto__' || + namespace === 'constructor' || + namespace === 'prototype' + ) { + throw new Error( + `Invalid namespace: "${namespace}" is not allowed.`, + ); + } + this.update((s) => { // Ensure the namespace bucket exists this.#ensureNamespaceBucket(s, namespace); diff --git a/packages/sample-controllers/src/sample-petnames-controller.ts b/packages/sample-controllers/src/sample-petnames-controller.ts index cf7a4a70784..c895e41da13 100644 --- a/packages/sample-controllers/src/sample-petnames-controller.ts +++ b/packages/sample-controllers/src/sample-petnames-controller.ts @@ -211,6 +211,14 @@ export class SamplePetnamesController extends BaseController< if (!isSafeDynamicKey(chainId)) { throw new Error('Invalid chain ID'); } + // Explicitly forbid keys that may cause prototype pollution. + if ( + chainId === '__proto__' || + chainId === 'constructor' || + chainId === 'prototype' + ) { + throw new Error('Unsafe chain ID'); + } const normalizedAddress = address.toLowerCase() as Hex;