Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/eth-json-rpc-middleware/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Add prototype pollution validation for `signTypedData` methods (V1, V3, V4) to block dangerous properties (`__proto__`, `constructor`, `prototype`, etc.) in message data. ([#7732](https://github.com/MetaMask/core/pull/7732))

### Changed

- Bump `@metamask/eth-block-tracker` from `^15.0.0` to `^15.0.1` ([#7642](https://github.com/MetaMask/core/pull/7642))
Expand Down
91 changes: 91 additions & 0 deletions packages/eth-json-rpc-middleware/src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { Struct, StructError } from '@metamask/superstruct';
import { validate } from '@metamask/superstruct';
import type { Hex } from '@metamask/utils';

import { parseTypedMessage } from './normalize';
import type { WalletMiddlewareContext } from '../wallet';

/**
Expand Down Expand Up @@ -96,3 +97,93 @@ function formatValidationError(error: StructError, message: string): string {
)
.join('\n')}`;
}

export const DANGEROUS_PROTOTYPE_PROPERTIES = [
'__proto__',
'constructor',
'prototype',
'__defineGetter__',
'__defineSetter__',
'__lookupGetter__',
'__lookupSetter__',
] as const;

/**
* Checks if a property name is dangerous for prototype pollution.
*
* @param key - The property name to check
* @returns True if the property name is dangerous
*/
function isDangerousProperty(key: string): boolean {
return (DANGEROUS_PROTOTYPE_PROPERTIES as readonly string[]).includes(key);
}

/**
* Recursively checks an object for dangerous prototype pollution properties.
*
* @param obj - The object to check
* @throws rpcErrors.invalidInput() if a dangerous property is found
*/
function checkObjectForPrototypePollution(obj: unknown): void {
if (obj === null || obj === undefined) {
return;
}

if (Array.isArray(obj)) {
for (const item of obj) {
checkObjectForPrototypePollution(item);
}
return;
}

if (typeof obj === 'object') {
for (const key of Object.getOwnPropertyNames(
obj as Record<string, unknown>,
)) {
if (isDangerousProperty(key)) {
throw rpcErrors.invalidInput();
}
checkObjectForPrototypePollution((obj as Record<string, unknown>)[key]);
}
}
}

/**
* Validates V1 typed data (array format) for prototype pollution attacks.
* V1 format: [{ type: 'string', name: 'fieldName', value: 'data' }, ...]
*
* @param data - The V1 typed data array to validate
* @throws rpcErrors.invalidInput() if prototype pollution is detected
*/
export function validateTypedDataV1ForPrototypePollution(
data: Record<string, unknown>[],
): void {
if (!data || !Array.isArray(data)) {
return;
}

for (const item of data) {
if (item && typeof item === 'object') {
// Only check the 'value' field (the message data) for dangerous properties
if (item.value !== null && typeof item.value === 'object') {
checkObjectForPrototypePollution(item.value);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V1 validation misses item-level dangerous properties

Medium Severity

The validateTypedDataV1ForPrototypePollution function only validates the value field within each item of the V1 typed data array, but doesn't check the item objects themselves for dangerous properties. An attacker could add properties like __proto__ or constructor directly on the item objects (alongside type, name, value), and these would not be detected. When these items are passed to processTypedMessage callback, downstream code that spreads or merges these objects could be vulnerable to prototype pollution.

Fix in Cursor Fix in Web

}
}

/**
* Validates V3/V4 typed data (EIP-712 format) for prototype pollution attacks.
* Only checks the message field for dangerous properties.
*
* @param data - The stringified typed data to validate
* @throws rpcErrors.invalidInput() if prototype pollution is detected
*/
export function validateTypedDataForPrototypePollution(data: string): void {
const { message } = parseTypedMessage(data);

// Check message recursively for dangerous properties
if (message !== undefined) {
checkObjectForPrototypePollution(message);
}
}
110 changes: 110 additions & 0 deletions packages/eth-json-rpc-middleware/src/wallet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
TypedMessageV1Params,
} from '.';
import { createWalletMiddleware } from '.';
import { DANGEROUS_PROTOTYPE_PROPERTIES } from './utils/validation';
import { createHandleParams, createRequest } from '../test/util/helpers';

const testAddresses = [
Expand Down Expand Up @@ -903,4 +904,113 @@ describe('wallet', () => {
expect(ecrecoverResult).toStrictEqual(signParams.addressHex);
});
});

describe('prototype pollution validation', () => {
describe('signTypedData (V1)', () => {
DANGEROUS_PROTOTYPE_PROPERTIES.forEach((dangerousProperty) => {
it(`should throw if value contains nested ${dangerousProperty}`, async () => {
const getAccounts = async (): Promise<string[]> =>
testAddresses.slice();
const processTypedMessage = async (): Promise<string> => testMsgSig;
const engine = JsonRpcEngineV2.create({
middleware: [
createWalletMiddleware({ getAccounts, processTypedMessage }),
],
});

const value = {};
Object.defineProperty(value, dangerousProperty, {
value: 'malicious',
enumerable: true,
});
const message = [{ type: 'object', name: 'data', value }];
const payload = {
method: 'eth_signTypedData',
params: [message, testAddresses[0]],
};

await expect(
engine.handle(...createHandleParams(payload)),
).rejects.toThrow('Invalid input.');
});
});
});

describe('signTypedDataV3', () => {
DANGEROUS_PROTOTYPE_PROPERTIES.forEach((dangerousProperty) => {
it(`should throw if message contains ${dangerousProperty}`, async () => {
const getAccounts = async (): Promise<string[]> =>
testAddresses.slice();
const processTypedMessageV3 = async (): Promise<string> => testMsgSig;
const engine = JsonRpcEngineV2.create({
middleware: [
createWalletMiddleware({ getAccounts, processTypedMessageV3 }),
],
});

const msgObj = {};
Object.defineProperty(msgObj, dangerousProperty, {
value: 'malicious',
enumerable: true,
});
const message = {
types: {
EIP712Domain: [{ name: 'name', type: 'string' }],
},
primaryType: 'EIP712Domain',
domain: {},
message: msgObj,
};

const payload = {
method: 'eth_signTypedData_v3',
params: [testAddresses[0], JSON.stringify(message)],
};

await expect(
engine.handle(...createHandleParams(payload)),
).rejects.toThrow('Invalid input.');
});
});
});

describe('signTypedDataV4', () => {
DANGEROUS_PROTOTYPE_PROPERTIES.forEach((dangerousProperty) => {
it(`should throw if message contains ${dangerousProperty}`, async () => {
const getAccounts = async (): Promise<string[]> =>
testAddresses.slice();
const processTypedMessageV4 = async (): Promise<string> => testMsgSig;
const engine = JsonRpcEngineV2.create({
middleware: [
createWalletMiddleware({ getAccounts, processTypedMessageV4 }),
],
});

const msgObj = {};
Object.defineProperty(msgObj, dangerousProperty, {
value: 'malicious',
enumerable: true,
});
const message = {
types: {
EIP712Domain: [{ name: 'name', type: 'string' }],
Permit: [{ name: 'owner', type: 'address' }],
},
primaryType: 'Permit',
domain: {},
message: msgObj,
};

const payload = {
method: 'eth_signTypedData_v4',
params: [testAddresses[0], JSON.stringify(message)],
};

await expect(
engine.handle(...createHandleParams(payload)),
).rejects.toThrow('Invalid input.');
});
});
});
});
});
5 changes: 5 additions & 0 deletions packages/eth-json-rpc-middleware/src/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import { normalizeTypedMessage, parseTypedMessage } from './utils/normalize';
import {
resemblesAddress,
validateAndNormalizeKeyholder as validateKeyholder,
validateTypedDataForPrototypePollution,
validateTypedDataV1ForPrototypePollution,
} from './utils/validation';

export type TransactionParams = {
Expand Down Expand Up @@ -323,6 +325,7 @@ export function createWalletMiddleware({
const message = params[0];
const address = await validateAndNormalizeKeyholder(params[1], context);
const version = 'V1';
validateTypedDataV1ForPrototypePollution(message);
// Not using nullish coalescing, since `params` may be `null`.
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const extraParams = params[2] || {};
Expand Down Expand Up @@ -366,6 +369,7 @@ export function createWalletMiddleware({
const message = normalizeTypedMessage(params[1]);
validatePrimaryType(message);
validateVerifyingContract(message);
validateTypedDataForPrototypePollution(message);
const version = 'V3';
const msgParams: TypedMessageParams = {
data: message,
Expand Down Expand Up @@ -406,6 +410,7 @@ export function createWalletMiddleware({
const message = normalizeTypedMessage(params[1]);
validatePrimaryType(message);
validateVerifyingContract(message);
validateTypedDataForPrototypePollution(message);
const version = 'V4';
const msgParams: TypedMessageParams = {
data: message,
Expand Down
Loading