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

Security provider check (OpenSea) (#16584)

This commit is contained in:
Filip Sekulic 2023-01-23 15:32:01 +01:00 committed by GitHub
parent d6cf809bcc
commit 27d34166fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 265 additions and 73 deletions

View File

@ -151,6 +151,7 @@ export default class TransactionController extends EventEmitter {
this.getDeviceModel = opts.getDeviceModel;
this.getAccountType = opts.getAccountType;
this.getTokenStandardAndDetails = opts.getTokenStandardAndDetails;
this.securityProviderRequest = opts.securityProviderRequest;
this.memStore = new ObservableStore({});
@ -337,6 +338,7 @@ export default class TransactionController extends EventEmitter {
);
const initialTxMeta = await this.addUnapprovedTransaction(
opts.method,
txParams,
opts.origin,
undefined,
@ -766,6 +768,7 @@ export default class TransactionController extends EventEmitter {
* actionId is fix used for making this action idempotent to deal with scenario when
* action is invoked multiple times with same parameters in MV3 due to service worker re-activation.
*
* @param txMethodType
* @param txParams
* @param origin
* @param transactionType
@ -774,6 +777,7 @@ export default class TransactionController extends EventEmitter {
* @returns {txMeta}
*/
async addUnapprovedTransaction(
txMethodType,
txParams,
origin,
transactionType,
@ -856,6 +860,15 @@ export default class TransactionController extends EventEmitter {
? addHexPrefix(txMeta.txParams.value)
: '0x0';
if (txMethodType && this.securityProviderRequest) {
const securityProviderResponse = await this.securityProviderRequest(
txMeta,
txMethodType,
);
txMeta.securityProviderResponse = securityProviderResponse;
}
this.addTransaction(txMeta);
this.emit('newUnapprovedTx', txMeta);

View File

@ -96,6 +96,7 @@ describe('Transaction Controller', function () {
getEIP1559GasFeeEstimates: () => undefined,
getAccountType: () => 'MetaMask',
getDeviceModel: () => 'N/A',
securityProviderRequest: () => undefined,
});
txController.nonceTracker.getNonceLock = () =>
Promise.resolve({ nextNonce: 0, releaseLock: noop });
@ -362,7 +363,7 @@ describe('Transaction Controller', function () {
});
it('should add an unapproved transaction and return a valid txMeta', async function () {
const txMeta = await txController.addUnapprovedTransaction({
const txMeta = await txController.addUnapprovedTransaction(undefined, {
from: selectedAddress,
to: recipientAddress,
});
@ -386,6 +387,7 @@ describe('Transaction Controller', function () {
it('should add only 1 unapproved transaction when called twice with same actionId', async function () {
await txController.addUnapprovedTransaction(
undefined,
{
from: selectedAddress,
to: recipientAddress,
@ -398,6 +400,7 @@ describe('Transaction Controller', function () {
const transactionCount1 =
txController.txStateManager.getTransactions().length;
await txController.addUnapprovedTransaction(
undefined,
{
from: selectedAddress,
to: recipientAddress,
@ -414,6 +417,7 @@ describe('Transaction Controller', function () {
it('should add multiple transactions when called with different actionId', async function () {
await txController.addUnapprovedTransaction(
undefined,
{
from: selectedAddress,
to: recipientAddress,
@ -426,6 +430,7 @@ describe('Transaction Controller', function () {
const transactionCount1 =
txController.txStateManager.getTransactions().length;
await txController.addUnapprovedTransaction(
undefined,
{
from: selectedAddress,
to: recipientAddress,
@ -447,7 +452,7 @@ describe('Transaction Controller', function () {
done();
});
txController
.addUnapprovedTransaction({
.addUnapprovedTransaction(undefined, {
from: selectedAddress,
to: recipientAddress,
})
@ -466,7 +471,7 @@ describe('Transaction Controller', function () {
networkStore.putState('loading');
await assert.rejects(
() =>
txController.addUnapprovedTransaction({
txController.addUnapprovedTransaction(undefined, {
from: selectedAddress,
to: '0x0d1d4e623D10F9FBA5Db95830F7d3839406C6AF2',
}),
@ -510,7 +515,7 @@ describe('Transaction Controller', function () {
});
it('should add an cancel transaction and return a valid txMeta', async function () {
const txMeta = await txController.addUnapprovedTransaction({
const txMeta = await txController.addUnapprovedTransaction(undefined, {
from: selectedAddress,
to: recipientAddress,
});
@ -528,7 +533,7 @@ describe('Transaction Controller', function () {
});
it('should add only 1 cancel transaction when called twice with same actionId', async function () {
const txMeta = await txController.addUnapprovedTransaction({
const txMeta = await txController.addUnapprovedTransaction(undefined, {
from: selectedAddress,
to: recipientAddress,
});
@ -551,7 +556,7 @@ describe('Transaction Controller', function () {
});
it('should add multiple transactions when called with different actionId', async function () {
const txMeta = await txController.addUnapprovedTransaction({
const txMeta = await txController.addUnapprovedTransaction(undefined, {
from: selectedAddress,
to: recipientAddress,
});
@ -1279,7 +1284,7 @@ describe('Transaction Controller', function () {
});
it('should add only 1 speedup transaction when called twice with same actionId', async function () {
const txMeta = await txController.addUnapprovedTransaction({
const txMeta = await txController.addUnapprovedTransaction(undefined, {
from: selectedAddress,
to: recipientAddress,
});
@ -1302,7 +1307,7 @@ describe('Transaction Controller', function () {
});
it('should add multiple transactions when called with different actionId', async function () {
const txMeta = await txController.addUnapprovedTransaction({
const txMeta = await txController.addUnapprovedTransaction(undefined, {
from: selectedAddress,
to: recipientAddress,
});
@ -1323,6 +1328,47 @@ describe('Transaction Controller', function () {
txController.txStateManager.getTransactions().length;
assert.equal(transactionCount1 + 1, transactionCount2);
});
it('should add multiple transactions when called with different actionId and txMethodType defined', async function () {
const txMeta = await txController.addUnapprovedTransaction(
'eth_sendTransaction',
{
from: selectedAddress,
to: recipientAddress,
},
);
await txController.approveTransaction(txMeta.id);
await txController.createSpeedUpTransaction(
txMeta.id,
{},
{ actionId: 12345 },
);
const transactionCount1 =
txController.txStateManager.getTransactions().length;
await txController.createSpeedUpTransaction(
txMeta.id,
{},
{ actionId: 11111 },
);
const transactionCount2 =
txController.txStateManager.getTransactions().length;
assert.equal(transactionCount1 + 1, transactionCount2);
});
it('should call securityProviderRequest and have flagAsDangerous inside txMeta', async function () {
const txMeta = await txController.addUnapprovedTransaction(
'eth_sendTransaction',
{
from: selectedAddress,
to: recipientAddress,
},
);
assert.ok(
'securityProviderResponse' in txMeta,
'should have a securityProviderResponse',
);
});
});
describe('#signTransaction', function () {

View File

@ -29,8 +29,9 @@ export default class MessageManager extends EventEmitter {
*
* @param {object} opts - Controller options
* @param {Function} opts.metricsEvent - A function for emitting a metric event.
* @param {Function} opts.securityProviderRequest - A function for verifying a message, whether it is malicious or not.
*/
constructor({ metricsEvent }) {
constructor({ metricsEvent, securityProviderRequest }) {
super();
this.memStore = new ObservableStore({
unapprovedMsgs: {},
@ -46,6 +47,7 @@ export default class MessageManager extends EventEmitter {
this.messages = [];
this.metricsEvent = metricsEvent;
this.securityProviderRequest = securityProviderRequest;
}
/**
@ -80,7 +82,7 @@ export default class MessageManager extends EventEmitter {
* @returns {promise} after signature has been
*/
async addUnapprovedMessageAsync(msgParams, req) {
const msgId = this.addUnapprovedMessage(msgParams, req);
const msgId = await this.addUnapprovedMessage(msgParams, req);
return await new Promise((resolve, reject) => {
// await finished
this.once(`${msgId}:finished`, (data) => {
@ -118,7 +120,7 @@ export default class MessageManager extends EventEmitter {
* @param {object} [req] - The original request object where the origin may be specified
* @returns {number} The id of the newly created message.
*/
addUnapprovedMessage(msgParams, req) {
async addUnapprovedMessage(msgParams, req) {
// add origin from request
if (req) {
msgParams.origin = req.origin;
@ -136,6 +138,13 @@ export default class MessageManager extends EventEmitter {
};
this.addMsg(msgData);
const securityProviderResponse = await this.securityProviderRequest(
msgData,
msgData.type,
);
msgData.securityProviderResponse = securityProviderResponse;
// signal update
this.emit('update');
return msgId;

View File

@ -36,8 +36,9 @@ export default class PersonalMessageManager extends EventEmitter {
*
* @param options
* @param options.metricsEvent
* @param options.securityProviderRequest
*/
constructor({ metricsEvent }) {
constructor({ metricsEvent, securityProviderRequest }) {
super();
this.memStore = new ObservableStore({
unapprovedPersonalMsgs: {},
@ -53,6 +54,7 @@ export default class PersonalMessageManager extends EventEmitter {
this.messages = [];
this.metricsEvent = metricsEvent;
this.securityProviderRequest = securityProviderRequest;
}
/**
@ -96,31 +98,32 @@ export default class PersonalMessageManager extends EventEmitter {
);
return;
}
const msgId = this.addUnapprovedMessage(msgParams, req);
this.once(`${msgId}:finished`, (data) => {
switch (data.status) {
case 'signed':
resolve(data.rawSig);
return;
case 'rejected':
reject(
ethErrors.provider.userRejectedRequest(
'MetaMask Message Signature: User denied message signature.',
),
);
return;
case 'errored':
reject(new Error(`MetaMask Message Signature: ${data.error}`));
return;
default:
reject(
new Error(
`MetaMask Message Signature: Unknown problem: ${JSON.stringify(
msgParams,
)}`,
),
);
}
this.addUnapprovedMessage(msgParams, req).then((msgId) => {
this.once(`${msgId}:finished`, (data) => {
switch (data.status) {
case 'signed':
resolve(data.rawSig);
return;
case 'rejected':
reject(
ethErrors.provider.userRejectedRequest(
'MetaMask Message Signature: User denied message signature.',
),
);
return;
case 'errored':
reject(new Error(`MetaMask Message Signature: ${data.error}`));
return;
default:
reject(
new Error(
`MetaMask Message Signature: Unknown problem: ${JSON.stringify(
msgParams,
)}`,
),
);
}
});
});
});
}
@ -134,7 +137,7 @@ export default class PersonalMessageManager extends EventEmitter {
* @param {object} [req] - The original request object possibly containing the origin
* @returns {number} The id of the newly created PersonalMessage.
*/
addUnapprovedMessage(msgParams, req) {
async addUnapprovedMessage(msgParams, req) {
log.debug(
`PersonalMessageManager addUnapprovedMessage: ${JSON.stringify(
msgParams,
@ -162,6 +165,13 @@ export default class PersonalMessageManager extends EventEmitter {
};
this.addMsg(msgData);
const securityProviderResponse = await this.securityProviderRequest(
msgData,
msgData.type,
);
msgData.securityProviderResponse = securityProviderResponse;
// signal update
this.emit('update');
return msgId;

View File

@ -0,0 +1,64 @@
import { MESSAGE_TYPE } from '../../../shared/constants/app';
export async function securityProviderCheck(
requestData,
methodName,
chainId,
currentLocale,
) {
let dataToValidate;
if (methodName === MESSAGE_TYPE.ETH_SIGN_TYPED_DATA) {
dataToValidate = {
host_name: requestData.msgParams.origin,
rpc_method_name: methodName,
chain_id: chainId,
data: requestData.msgParams.data,
currentLocale,
};
} else if (
methodName === MESSAGE_TYPE.ETH_SIGN ||
methodName === MESSAGE_TYPE.PERSONAL_SIGN
) {
dataToValidate = {
host_name: requestData.msgParams.origin,
rpc_method_name: methodName,
chain_id: chainId,
data: {
signer_address: requestData.msgParams.from,
msg_to_sign: requestData.msgParams.data,
},
currentLocale,
};
} else {
dataToValidate = {
host_name: requestData.origin,
rpc_method_name: methodName,
chain_id: chainId,
data: {
from_address: requestData.txParams.from,
to_address: requestData.txParams.to,
gas: requestData.txParams.gas,
gasPrice: requestData.txParams.gasPrice,
value: requestData.txParams.value,
data: requestData.txParams.data,
},
currentLocale,
};
}
const response = await fetch(
'https://eos9d7dmfj.execute-api.us-east-1.amazonaws.com/metamask/validate',
{
method: 'POST',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
'X-API-KEY': 'NKYIN6cXkFaNnVIfzNx7s1z0p3b0B4SB6k29qA7n',
},
body: JSON.stringify(dataToValidate),
},
);
return await response.json();
}

View File

@ -35,8 +35,9 @@ export default class TypedMessageManager extends EventEmitter {
* @param options
* @param options.getCurrentChainId
* @param options.metricsEvent
* @param options.securityProviderRequest
*/
constructor({ getCurrentChainId, metricsEvent }) {
constructor({ getCurrentChainId, metricsEvent, securityProviderRequest }) {
super();
this._getCurrentChainId = getCurrentChainId;
this.memStore = new ObservableStore({
@ -53,6 +54,7 @@ export default class TypedMessageManager extends EventEmitter {
this.messages = [];
this.metricsEvent = metricsEvent;
this.securityProviderRequest = securityProviderRequest;
}
/**
@ -89,32 +91,33 @@ export default class TypedMessageManager extends EventEmitter {
* @param version
* @returns {promise} When the message has been signed or rejected
*/
addUnapprovedMessageAsync(msgParams, req, version) {
async addUnapprovedMessageAsync(msgParams, req, version) {
return new Promise((resolve, reject) => {
const msgId = this.addUnapprovedMessage(msgParams, req, version);
this.once(`${msgId}:finished`, (data) => {
switch (data.status) {
case 'signed':
return resolve(data.rawSig);
case 'rejected':
return reject(
ethErrors.provider.userRejectedRequest(
'MetaMask Message Signature: User denied message signature.',
),
);
case 'errored':
return reject(
new Error(`MetaMask Message Signature: ${data.error}`),
);
default:
return reject(
new Error(
`MetaMask Message Signature: Unknown problem: ${JSON.stringify(
msgParams,
)}`,
),
);
}
this.addUnapprovedMessage(msgParams, req, version).then((msgId) => {
this.once(`${msgId}:finished`, (data) => {
switch (data.status) {
case 'signed':
return resolve(data.rawSig);
case 'rejected':
return reject(
ethErrors.provider.userRejectedRequest(
'MetaMask Message Signature: User denied message signature.',
),
);
case 'errored':
return reject(
new Error(`MetaMask Message Signature: ${data.error}`),
);
default:
return reject(
new Error(
`MetaMask Message Signature: Unknown problem: ${JSON.stringify(
msgParams,
)}`,
),
);
}
});
});
});
}
@ -129,7 +132,7 @@ export default class TypedMessageManager extends EventEmitter {
* @param version
* @returns {number} The id of the newly created TypedMessage.
*/
addUnapprovedMessage(msgParams, req, version) {
async addUnapprovedMessage(msgParams, req, version) {
msgParams.version = version;
if (req) {
msgParams.origin = req.origin;
@ -152,6 +155,13 @@ export default class TypedMessageManager extends EventEmitter {
};
this.addMsg(msgData);
const securityProviderResponse = await this.securityProviderRequest(
msgData,
msgData.type,
);
msgData.securityProviderResponse = securityProviderResponse;
// signal update
this.emit('update');
return msgId;

View File

@ -17,6 +17,7 @@ describe('Typed Message Manager', () => {
typedMessageManager = new TypedMessageManager({
getCurrentChainId: sinon.fake.returns('0x1'),
metricsEvent: sinon.fake(),
securityProviderRequest: sinon.fake(),
});
msgParamsV1 = {
@ -80,8 +81,8 @@ describe('Typed Message Manager', () => {
numberMsgId = parseInt(msgId, 10);
});
it('supports version 1 of signedTypedData', () => {
typedMessageManager.addUnapprovedMessage(msgParamsV1, null, 'V1');
it('supports version 1 of signedTypedData', async () => {
await typedMessageManager.addUnapprovedMessage(msgParamsV1, null, 'V1');
expect(messages[messages.length - 1].msgParams.data).toStrictEqual(
msgParamsV1.data,
);

View File

@ -244,6 +244,7 @@ describe('MetaMaskController', function () {
await metamaskController.createNewVaultAndKeychain('test@123');
const accounts = await metamaskController.keyringController.getAccounts();
const txMeta = await metamaskController.getApi().addUnapprovedTransaction(
undefined,
{
from: accounts[0],
to: recipientAddress,

View File

@ -173,6 +173,7 @@ import createRPCMethodTrackingMiddleware from './lib/createRPCMethodTrackingMidd
import { checkSnapsBlockList } from './flask/snaps-utilities';
import { SNAP_BLOCKLIST } from './flask/snaps-blocklist';
///: END:ONLY_INCLUDE_IN
import { securityProviderCheck } from './lib/security-provider-helpers';
export const METAMASK_CONTROLLER_EVENTS = {
// Fired after state changes that impact the extension badge (unapproved msg count)
@ -927,6 +928,7 @@ export default class MetamaskController extends EventEmitter {
this.assetsContractController.getTokenStandardAndDetails.bind(
this.assetsContractController,
),
securityProviderRequest: this.securityProviderRequest.bind(this),
});
this.txController.on('newUnapprovedTx', () => opts.showUserConfirmation());
@ -1024,11 +1026,13 @@ export default class MetamaskController extends EventEmitter {
metricsEvent: this.metaMetricsController.trackEvent.bind(
this.metaMetricsController,
),
securityProviderRequest: this.securityProviderRequest.bind(this),
});
this.personalMessageManager = new PersonalMessageManager({
metricsEvent: this.metaMetricsController.trackEvent.bind(
this.metaMetricsController,
),
securityProviderRequest: this.securityProviderRequest.bind(this),
});
this.decryptMessageManager = new DecryptMessageManager({
metricsEvent: this.metaMetricsController.trackEvent.bind(
@ -1047,6 +1051,7 @@ export default class MetamaskController extends EventEmitter {
metricsEvent: this.metaMetricsController.trackEvent.bind(
this.metaMetricsController,
),
securityProviderRequest: this.securityProviderRequest.bind(this),
});
this.swapsController = new SwapsController({
@ -4571,4 +4576,31 @@ export default class MetamaskController extends EventEmitter {
}
}
};
async securityProviderRequest(requestData, methodName) {
const { currentLocale, transactionSecurityCheckEnabled } =
this.preferencesController.store.getState();
const chainId = Number(
hexToDecimal(this.networkController.getCurrentChainId()),
);
if (transactionSecurityCheckEnabled) {
try {
const securityProviderResponse = await securityProviderCheck(
requestData,
methodName,
chainId,
currentLocale,
);
return securityProviderResponse;
} catch (err) {
log.error(err.message);
throw err;
}
}
return null;
}
}

View File

@ -2291,6 +2291,7 @@ export function signTransaction() {
dispatch(
addUnapprovedTransactionAndRouteToConfirmationPage(
undefined,
txParams,
transactionType,
draftTransaction.history,

View File

@ -93,7 +93,7 @@ jest.mock('lodash', () => ({
setBackgroundConnection({
addPollingTokenToAppState: jest.fn(),
addUnapprovedTransaction: jest.fn((_v, _w, _x, _y, _z, cb) => {
addUnapprovedTransaction: jest.fn((_u, _v, _w, _x, _y, _z, cb) => {
cb(null);
}),
updateTransactionSendFlowHistory: jest.fn((_x, _y, _z, cb) => cb(null)),
@ -2316,13 +2316,13 @@ describe('Send Slice', () => {
expect(
addUnapprovedTransactionAndRouteToConfirmationPageStub.mock
.calls[0][0].data,
.calls[0][1].data,
).toStrictEqual(
'0xa9059cbb0000000000000000000000004f90e18605fd46f9f9fab0e225d88e1acf5f53240000000000000000000000000000000000000000000000000000000000000001',
);
expect(
addUnapprovedTransactionAndRouteToConfirmationPageStub.mock
.calls[0][0].to,
.calls[0][1].to,
).toStrictEqual('0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2');
});
});

View File

@ -1202,6 +1202,7 @@ export const signAndSendTransactions = (
delete approveTxParams.gasPrice;
}
const approveTxMeta = await addUnapprovedTransaction(
undefined,
{ ...approveTxParams, amount: '0x0' },
TransactionType.swapApproval,
);
@ -1222,6 +1223,7 @@ export const signAndSendTransactions = (
}
const tradeTxMeta = await addUnapprovedTransaction(
undefined,
usedTradeTxParams,
TransactionType.swap,
);

View File

@ -880,6 +880,7 @@ export function updateTransaction(txData, dontShowLoadingIndicator) {
* confirmation page. Returns the newly created txMeta in case additional logic
* should be applied to the transaction after creation.
*
* @param method
* @param {import('../../shared/constants/transaction').TxParams} txParams -
* The transaction parameters
* @param {import(
@ -890,6 +891,7 @@ export function updateTransaction(txData, dontShowLoadingIndicator) {
* @returns {import('../../shared/constants/transaction').TransactionMeta}
*/
export function addUnapprovedTransactionAndRouteToConfirmationPage(
method,
txParams,
type,
sendFlowHistory,
@ -900,7 +902,7 @@ export function addUnapprovedTransactionAndRouteToConfirmationPage(
log.debug('background.addUnapprovedTransaction');
const txMeta = await submitRequestToBackground(
'addUnapprovedTransaction',
[txParams, ORIGIN_METAMASK, type, sendFlowHistory, actionId],
[method, txParams, ORIGIN_METAMASK, type, sendFlowHistory, actionId],
actionId,
);
dispatch(showConfTxPage());
@ -919,6 +921,7 @@ export function addUnapprovedTransactionAndRouteToConfirmationPage(
* This method does not show errors or route to a confirmation page and is
* used primarily for swaps functionality.
*
* @param method
* @param {import('../../shared/constants/transaction').TxParams} txParams -
* The transaction parameters
* @param {import(
@ -926,12 +929,12 @@ export function addUnapprovedTransactionAndRouteToConfirmationPage(
* ).TransactionType} type - The type of the transaction being added.
* @returns {import('../../shared/constants/transaction').TransactionMeta}
*/
export async function addUnapprovedTransaction(txParams, type) {
export async function addUnapprovedTransaction(method, txParams, type) {
log.debug('background.addUnapprovedTransaction');
const actionId = generateActionId();
const txMeta = await submitRequestToBackground(
'addUnapprovedTransaction',
[txParams, ORIGIN_METAMASK, type, undefined, actionId],
[method, txParams, ORIGIN_METAMASK, type, undefined, actionId],
actionId,
);
return txMeta;