m/metamask-extension
1
0
Fork 0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-12-23 09:52:26 +01:00
Code

"eth_signTypedData" presents fields that do not appear in 'types' filed (#12905)

Browse Source 
* Premilimary Sanitize data logic.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* sanitizeData v2

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* sanitizeData: take 3

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Sanitize Data take 4

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Lint fixes

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Check that version is v4 before sanitizing.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* sanitize arrays.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Tests to check that typeless data are not shwon

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Lint Fixes

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Do not check value types, Iterate through the message, and ensure each property of the message is declared as a type

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Check that if data type is not defined, it is a solidity type.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Lint Fixes

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Code cleanup

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Move sanitizeData to utils
Tests for sanitizeData in utils

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Lint fixes

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Fix unit tests for signaturerequest

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Remove unused type
include fixedMxN and ufixedMxN checks.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* move fixtures to before each

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* invert if condition to avoid indentations.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Lint fixes

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* We should exclude types with [] at the beginning or middle as well:

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* cache nestedType

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Throw error for undefined/invalid types definition

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>

* Throw if base type and types are not defined.

Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
This commit is contained in:
Olusegun Akintayo 2021-12-09 00:56:09 +04:00 committed by GitHub
parent d2843c3bb4
commit a7da8333a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 307 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
ui/components/app/signature-request/signature-request.component.js
View File

@ -2,6 +2,7 @@ import React, { PureComponent } from 'react';
import PropTypes from 'prop-types';
import Identicon from '../../ui/identicon';
import LedgerInstructionField from '../ledger-instruction-field';
import { sanitizeMessage } from '../../../helpers/utils/util';
import Header from './signature-request-header';
import Footer from './signature-request-footer';
import Message from './signature-request-message';
@ -63,7 +64,7 @@ export default class SignatureRequest extends PureComponent {
hardwareWalletRequiresConnection,
} = this.props;
const { address: fromAddress } = fromAccount;
const { message, domain = {} } = JSON.parse(data);
const { message, domain = {}, primaryType, types } = JSON.parse(data);
const { metricsEvent } = this.context;
const onSign = (event) => {
@ -123,7 +124,7 @@ export default class SignatureRequest extends PureComponent {
<LedgerInstructionField showDataInstruction />
</div>
) : null}
<Message data={message} />
<Message data={sanitizeMessage(message, primaryType, types)} />
<Footer
cancelAction={onCancel}
signAction={onSign}

121
ui/components/app/signature-request/signature-request.component.test.js
View File

@ -1,21 +1,77 @@
import React from 'react';
import { shallowWithContext } from '../../../../test/lib/render-helpers';
import SignatureRequest from './signature-request.component';
import Message from './signature-request-message';
describe('Signature Request Component', () => {
describe('render', () => {
const fromAddress = '0x123456789abcdef';
it('should render a div with one child', () => {
let fromAddress;
let messageData;
beforeEach(() => {
fromAddress = '0x123456789abcdef';
messageData = {
domain: {
chainId: 97,
name: 'Ether Mail',
verifyingContract: '0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC',
version: '1',
},
message: {
contents: 'Hello, Bob!',
from: {
name: 'Cow',
wallets: [
'0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826',
'0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF',
],
},
to: [
{
name: 'Bob',
wallets: [
'0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB',
'0xB0BdaBea57B0BDABeA57b0bdABEA57b0BDabEa57',
'0xB0B0b0b0b0b0B000000000000000000000000000',
],
},
],
},
primaryType: 'Mail',
types: {
EIP712Domain: [
{ name: 'name', type: 'string' },
{ name: 'version', type: 'string' },
{ name: 'chainId', type: 'uint256' },
{ name: 'verifyingContract', type: 'address' },
],
Mail: [
{ name: 'from', type: 'Person' },
{ name: 'to', type: 'Person[]' },
{ name: 'contents', type: 'string' },
],
Person: [
{ name: 'name', type: 'string' },
{ name: 'wallets', type: 'address[]' },
],
},
};
});
it('should render a div message parsed', () => {
const msgParams = {
data: JSON.stringify(messageData),
version: 'V4',
origin: 'test',
};
const wrapper = shallowWithContext(
<SignatureRequest
hardwareWalletRequiresConnection={() => false}
clearConfirmTransaction={() => undefined}
cancel={() => undefined}
sign={() => undefined}
txData={{
msgParams: {
data: '{"message": {"from": {"name": "hello"}}}',
from: fromAddress,
},
msgParams,
}}
fromAccount={{ address: fromAddress }}
/>,
@ -24,6 +80,59 @@ describe('Signature Request Component', () => {
expect(wrapper.is('div')).toStrictEqual(true);
expect(wrapper).toHaveLength(1);
expect(wrapper.hasClass('signature-request')).toStrictEqual(true);
const messageWrapper = wrapper.find(Message);
expect(messageWrapper).toHaveLength(1);
const { data } = messageWrapper.props();
expect(data.contents).toStrictEqual('Hello, Bob!');
expect(data.from.name).toStrictEqual('Cow');
expect(data.from.wallets).toBeDefined();
expect(data.from.wallets).toHaveLength(2);
expect(data.to).toBeDefined();
const dataTo = data.to;
expect(dataTo[0].name).toStrictEqual('Bob');
expect(dataTo[0].wallets).toHaveLength(3);
});
it('should render a div message parsed without typeless data', () => {
messageData.message.do_not_display = 'one';
messageData.message.do_not_display_2 = {
do_not_display: 'two',
};
const msgParams = {
data: JSON.stringify(messageData),
version: 'V4',
origin: 'test',
};
const wrapper = shallowWithContext(
<SignatureRequest
hardwareWalletRequiresConnection={() => false}
clearConfirmTransaction={() => undefined}
cancel={() => undefined}
sign={() => undefined}
txData={{
msgParams,
}}
fromAccount={{ address: fromAddress }}
/>,
);
expect(wrapper.is('div')).toStrictEqual(true);
expect(wrapper).toHaveLength(1);
expect(wrapper.hasClass('signature-request')).toStrictEqual(true);
const messageWrapper = wrapper.find(Message);
expect(messageWrapper).toHaveLength(1);
const { data } = messageWrapper.props();
expect(data.contents).toStrictEqual('Hello, Bob!');
expect(data.from.name).toStrictEqual('Cow');
expect(data.from.wallets).toBeDefined();
expect(data.from.wallets).toHaveLength(2);
expect(data.to).toBeDefined();
const dataTo = data.to;
expect(dataTo[0].name).toStrictEqual('Bob');
expect(dataTo[0].wallets).toHaveLength(3);
expect(data.do_not_display).toBeUndefined();
expect(data.do_not_display2).toBeUndefined();
});
});
});

103
ui/helpers/utils/util.js
View File

@ -443,3 +443,106 @@ export const toHumanReadableTime = (t, milliseconds) => {
export function clearClipboard() {
window.navigator.clipboard.writeText('');
}
const solidityTypes = () => {
const types = [
'bool',
'address',
'string',
'bytes',
'int',
'uint',
'fixed',
'ufixed',
];
const ints = Array.from(new Array(32)).map(
(_, index) => `int${(index + 1) * 8}`,
);
const uints = Array.from(new Array(32)).map(
(_, index) => `uint${(index + 1) * 8}`,
);
const bytes = Array.from(new Array(32)).map(
(_, index) => `bytes${index + 1}`,
);
/**
* fixed and ufixed
* This value type also can be declared keywords such as ufixedMxN and fixedMxN.
* The M represents the amount of bits that the type takes,
* with N representing the number of decimal points that are available.
* M has to be divisible by 8, and a number from 8 to 256.
* N has to be a value between 0 and 80, also being inclusive.
*/
const fixedM = Array.from(new Array(32)).map(
(_, index) => `fixed${(index + 1) * 8}`,
);
const ufixedM = Array.from(new Array(32)).map(
(_, index) => `ufixed${(index + 1) * 8}`,
);
const fixed = Array.from(new Array(80)).map((_, index) =>
fixedM.map((aFixedM) => `${aFixedM}x${index + 1}`),
);
const ufixed = Array.from(new Array(80)).map((_, index) =>
ufixedM.map((auFixedM) => `${auFixedM}x${index + 1}`),
);
return [
...types,
...ints,
...uints,
...bytes,
...fixed.flat(),
...ufixed.flat(),
];
};
export const sanitizeMessage = (msg, baseType, types) => {
if (!types) {
throw new Error(`Invalid types definition`);
}
const baseTypeDefinitions = types[baseType];
if (!baseTypeDefinitions) {
throw new Error(`Invalid primary type definition`);
}
const sanitizedMessage = {};
const msgKeys = Object.keys(msg);
msgKeys.forEach((msgKey) => {
const definedType = Object.values(baseTypeDefinitions).find(
(baseTypeDefinition) => baseTypeDefinition.name === msgKey,
);
if (!definedType) {
return;
}
// key has a type. check if the definedType is also a type
const nestedType = definedType.type.replace(/\[\]$/u, '');
const nestedTypeDefinition = types[nestedType];
if (nestedTypeDefinition) {
if (definedType.type.endsWith('[]') > 0) {
// nested array
sanitizedMessage[msgKey] = msg[msgKey].map((value) =>
sanitizeMessage(value, nestedType, types),
);
} else {
// nested object
sanitizedMessage[msgKey] = sanitizeMessage(
msg[msgKey],
definedType.type,
types,
);
}
} else {
// check if it's a valid solidity type
const isSolidityType = solidityTypes().includes(nestedType);
if (isSolidityType) {
sanitizedMessage[msgKey] = msg[msgKey];
}
}
});
return sanitizedMessage;
};

86
ui/helpers/utils/util.test.js
View File

@ -340,4 +340,90 @@ describe('util', () => {
expect(util.toHumanReadableTime(t, 7200000)).toStrictEqual('2 hrs');
});
});
describe('sanitizeMessage', () => {
let message;
let primaryType;
let types;
beforeEach(() => {
message = {
contents: 'Hello, Bob!',
from: {
name: 'Cow',
wallets: [
'0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826',
'0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF',
],
},
to: [
{
name: 'Bob',
wallets: [
'0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB',
'0xB0BdaBea57B0BDABeA57b0bdABEA57b0BDabEa57',
'0xB0B0b0b0b0b0B000000000000000000000000000',
],
},
],
};
primaryType = 'Mail';
types = {
EIP712Domain: [
{ name: 'name', type: 'string' },
{ name: 'version', type: 'string' },
{ name: 'chainId', type: 'uint256' },
{ name: 'verifyingContract', type: 'address' },
],
Mail: [
{ name: 'from', type: 'Person' },
{ name: 'to', type: 'Person[]' },
{ name: 'contents', type: 'string' },
],
Person: [
{ name: 'name', type: 'string' },
{ name: 'wallets', type: 'address[]' },
],
};
});
it('should throw an error if types is undefined', () => {
expect(() =>
util.sanitizeMessage(message, primaryType, undefined),
).toThrow('Invalid types definition');
});
it('should throw an error if base type is not defined', () => {
expect(() => util.sanitizeMessage(message, undefined, types)).toThrow(
'Invalid primary type definition',
);
});
it('should return parsed message if types is defined', () => {
const result = util.sanitizeMessage(message, primaryType, types);
expect(result.contents).toStrictEqual('Hello, Bob!');
expect(result.from.name).toStrictEqual('Cow');
expect(result.from.wallets).toHaveLength(2);
expect(result.to).toHaveLength(1);
expect(result.to[0].name).toStrictEqual('Bob');
expect(result.to[0].wallets).toHaveLength(3);
});
it('should return ignore message data with unknown types', () => {
message.do_not_display = 'one';
message.do_not_display_2 = {
do_not_display: 'two',
};
// result will NOT contain the do_not_displays because type definition
const result = util.sanitizeMessage(message, primaryType, types);
expect(result.contents).toStrictEqual('Hello, Bob!');
expect(result.from.name).toStrictEqual('Cow');
expect(result.from.wallets).toHaveLength(2);
expect(result.to).toHaveLength(1);
expect(result.to[0].name).toStrictEqual('Bob');
expect(result.to[0].wallets).toHaveLength(3);
expect(result.do_not_display).toBeUndefined();
expect(result.do_not_display_2).toBeUndefined();
});
});
});