Skip to content

Commit 126626d

Browse files
authored
fix: Add validation for types sign message primary type (#350)
1 parent 4de470d commit 126626d

File tree

4 files changed

+108
-0
lines changed

4 files changed

+108
-0
lines changed

src/utils/common.test.ts

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { stripArrayTypeIfPresent } from './common';
2+
3+
describe('CommonUtils', () => {
4+
describe('stripArrayTypeIfPresent', () => {
5+
it('remove array brackets from the type if present', () => {
6+
expect(stripArrayTypeIfPresent('string[]')).toBe('string');
7+
expect(stripArrayTypeIfPresent('string[5]')).toBe('string');
8+
});
9+
10+
it('return types which are not array without any change', () => {
11+
expect(stripArrayTypeIfPresent('string')).toBe('string');
12+
expect(stripArrayTypeIfPresent('string []')).toBe('string []');
13+
});
14+
});
15+
});

src/utils/common.ts

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* Function to stripe array brackets if string defining the type has it.
3+
*
4+
* @param typeString - String defining type from which array brackets are required to be removed.
5+
* @returns Parameter string with array brackets [] removed.
6+
*/
7+
export const stripArrayTypeIfPresent = (typeString: string) => {
8+
if (typeString?.match(/\S\[\d*\]$/u) !== null) {
9+
return typeString.replace(/\[\d*\]$/gu, '').trim();
10+
}
11+
return typeString;
12+
};

src/wallet.test.ts

+57
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,63 @@ describe('wallet', () => {
626626
'0x68dc980608bceb5f99f691e62c32caccaee05317309015e9454eba1a14c3cd4505d1dd098b8339801239c9bcaac3c4df95569dcf307108b92f68711379be14d81c',
627627
});
628628
});
629+
630+
it('should throw if message does not have types defined', async () => {
631+
const { engine } = createTestSetup();
632+
const getAccounts = async () => testAddresses.slice();
633+
const witnessedMsgParams: TypedMessageParams[] = [];
634+
const processTypedMessageV4 = async (msgParams: TypedMessageParams) => {
635+
witnessedMsgParams.push(msgParams);
636+
// Assume testMsgSig is the expected signature result
637+
return testMsgSig;
638+
};
639+
640+
engine.push(
641+
createWalletMiddleware({ getAccounts, processTypedMessageV4 }),
642+
);
643+
644+
const messageParams = getMsgParams();
645+
const payload = {
646+
method: 'eth_signTypedData_v4',
647+
params: [
648+
testAddresses[0],
649+
JSON.stringify({ ...messageParams, types: undefined }),
650+
],
651+
};
652+
653+
const promise = pify(engine.handle).call(engine, payload);
654+
await expect(promise).rejects.toThrow('Invalid input.');
655+
});
656+
657+
it('should throw if type of primaryType is not defined', async () => {
658+
const { engine } = createTestSetup();
659+
const getAccounts = async () => testAddresses.slice();
660+
const witnessedMsgParams: TypedMessageParams[] = [];
661+
const processTypedMessageV4 = async (msgParams: TypedMessageParams) => {
662+
witnessedMsgParams.push(msgParams);
663+
// Assume testMsgSig is the expected signature result
664+
return testMsgSig;
665+
};
666+
667+
engine.push(
668+
createWalletMiddleware({ getAccounts, processTypedMessageV4 }),
669+
);
670+
671+
const messageParams = getMsgParams();
672+
const payload = {
673+
method: 'eth_signTypedData_v4',
674+
params: [
675+
testAddresses[0],
676+
JSON.stringify({
677+
...messageParams,
678+
types: { ...messageParams.types, Permit: undefined },
679+
}),
680+
],
681+
};
682+
683+
const promise = pify(engine.handle).call(engine, payload);
684+
await expect(promise).rejects.toThrow('Invalid input.');
685+
});
629686
});
630687

631688
describe('sign', () => {

src/wallet.ts

+24
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
} from '@metamask/utils';
1414

1515
import type { Block } from './types';
16+
import { stripArrayTypeIfPresent } from './utils/common';
1617
import { normalizeTypedMessage, parseTypedMessage } from './utils/normalize';
1718

1819
/*
@@ -243,6 +244,7 @@ WalletMiddlewareOptions): JsonRpcMiddleware<any, Block> {
243244

244245
const address = await validateAndNormalizeKeyholder(params[0], req);
245246
const message = normalizeTypedMessage(params[1]);
247+
validatePrimaryType(message);
246248
validateVerifyingContract(message);
247249
const version = 'V3';
248250
const msgParams: TypedMessageParams = {
@@ -274,6 +276,7 @@ WalletMiddlewareOptions): JsonRpcMiddleware<any, Block> {
274276

275277
const address = await validateAndNormalizeKeyholder(params[0], req);
276278
const message = normalizeTypedMessage(params[1]);
279+
validatePrimaryType(message);
277280
validateVerifyingContract(message);
278281
const version = 'V4';
279282
const msgParams: TypedMessageParams = {
@@ -457,6 +460,27 @@ WalletMiddlewareOptions): JsonRpcMiddleware<any, Block> {
457460
}
458461
}
459462

463+
/**
464+
* Validates primary of typedSignMessage, to ensure that it's type definition is present in message.
465+
*
466+
* @param data - The data passed in typedSign request.
467+
*/
468+
function validatePrimaryType(data: string) {
469+
const { primaryType, types } = parseTypedMessage(data);
470+
if (!types) {
471+
throw rpcErrors.invalidInput();
472+
}
473+
474+
// Primary type can be an array.
475+
const baseType = stripArrayTypeIfPresent(primaryType);
476+
477+
// Return if the base type is not defined in the types
478+
const baseTypeDefinitions = types[baseType];
479+
if (!baseTypeDefinitions) {
480+
throw rpcErrors.invalidInput();
481+
}
482+
}
483+
460484
/**
461485
* Validates verifyingContract of typedSignMessage.
462486
*

0 commit comments

Comments
 (0)