diff --git a/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch b/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch similarity index 78% rename from .yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch rename to .yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch index 956b9fe308f7..4b9ccd4f64d9 100644 --- a/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch +++ b/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch @@ -1,5 +1,5 @@ diff --git a/dist/KeyringController.d.ts b/dist/KeyringController.d.ts -index ccfd5ca9accf782b0a612ab1666501898cf2abb9..f772552156491b308f2aa29a734a5138237d951e 100644 +index d6d70eea2de56310b49fa9dcf1e90afae3ddb202..4518aed8e03376d76e5ab6ef0971529e65563fd3 100644 --- a/dist/KeyringController.d.ts +++ b/dist/KeyringController.d.ts @@ -1,10 +1,11 @@ @@ -10,13 +10,12 @@ index ccfd5ca9accf782b0a612ab1666501898cf2abb9..f772552156491b308f2aa29a734a5138 import { BaseControllerV2 } from '@metamask/base-controller'; import type { PersonalMessageParams, TypedMessageParams } from '@metamask/message-manager'; import type { PreferencesController } from '@metamask/preferences-controller'; --import { type Hex, type Keyring, type Json } from '@metamask/utils'; -+import type { Hex, Keyring, Json } from '@metamask/utils'; + import type { Eip1024EncryptedData, Hex, Keyring, Json } from '@metamask/utils'; +import type { KeyringController as EthKeyringController } from '@metamask/eth-keyring-controller'; import type { Patch } from 'immer'; declare const name = "KeyringController"; /** -@@ -135,6 +136,10 @@ export declare class KeyringController extends BaseControllerV2() => }, } as any as jest.Mocked); -describe('EncryptionPublicKeyController', () => { +describe('DecryptMessageController', () => { let decryptMessageController: DecryptMessageController; const decryptMessageManagerConstructorMock = @@ -65,6 +67,19 @@ describe('EncryptionPublicKeyController', () => { const decryptMessageManagerMock = createDecryptMessageManagerMock(); + const mockMessengerAction = ( + action: string, + callback: (actionName: string, ...args: any[]) => any, + ) => { + messengerMock.call.mockImplementation((actionName, ...rest) => { + if (actionName === action) { + return callback(actionName, ...rest); + } + + return Promise.resolve(); + }); + }; + beforeEach(() => { jest.resetAllMocks(); @@ -116,12 +131,18 @@ describe('EncryptionPublicKeyController', () => { it('should decrypt message', async () => { const messageToDecrypt = { ...messageMock, - data: '0x7b22666f6f223a22626172227d', + data: messageDataMock, }; + const decryptMessageActionCallbackMock = jest + .fn() + .mockReturnValue('decryptedMessage'); decryptMessageManagerMock.approveMessage.mockResolvedValue( messageToDecrypt, ); - keyringControllerMock.decryptMessage.mockResolvedValue('decryptedMessage'); + mockMessengerAction( + 'KeyringController:decryptMessage', + decryptMessageActionCallbackMock, + ); getStateMock.mockReturnValue(mockExtState); const result = await decryptMessageController.decryptMessage( @@ -132,8 +153,9 @@ describe('EncryptionPublicKeyController', () => { expect(decryptMessageManagerMock.approveMessage).toBeCalledWith( messageToDecrypt, ); - expect(keyringControllerMock.decryptMessage).toBeCalledTimes(1); - expect(keyringControllerMock.decryptMessage).toBeCalledWith( + expect(decryptMessageActionCallbackMock).toBeCalledTimes(1); + expect(decryptMessageActionCallbackMock).toBeCalledWith( + 'KeyringController:decryptMessage', messageToDecrypt, ); expect(decryptMessageManagerMock.setMessageStatusAndResult).toBeCalledTimes( @@ -147,15 +169,31 @@ describe('EncryptionPublicKeyController', () => { expect(result).toBe(mockExtState); }); + it('should throw when decrypting invalid message', async () => { + const messageToDecrypt = { + ...messageMock, + data: '0x7b2022666f6f223a202262617222207d', + }; + decryptMessageManagerMock.approveMessage.mockResolvedValue( + messageToDecrypt, + ); + + expect( + decryptMessageController.decryptMessage(messageToDecrypt), + ).rejects.toThrow('Invalid encrypted data.'); + }); + it('should cancel decrypt request', async () => { const messageToDecrypt = { ...messageMock, - data: '0x7b22666f6f223a22626172227d', + data: messageDataMock, }; decryptMessageManagerMock.approveMessage.mockResolvedValue( messageToDecrypt, ); - keyringControllerMock.decryptMessage.mockRejectedValue(new Error('error')); + mockMessengerAction('KeyringController:decryptMessage', async () => { + throw new Error('error'); + }); getStateMock.mockReturnValue(mockExtState); return expect( @@ -166,10 +204,13 @@ describe('EncryptionPublicKeyController', () => { it('should decrypt message inline', async () => { const messageToDecrypt = { ...messageMock, - data: '0x7b22666f6f223a22626172227d', + data: messageDataMock, }; decryptMessageManagerMock.getMessage.mockReturnValue(messageToDecrypt); - keyringControllerMock.decryptMessage.mockResolvedValue('decryptedMessage'); + mockMessengerAction( + 'KeyringController:decryptMessage', + async () => 'decryptedMessage', + ); getStateMock.mockReturnValue(mockExtState); const result = await decryptMessageController.decryptMessageInline( @@ -184,6 +225,17 @@ describe('EncryptionPublicKeyController', () => { expect(result).toBe(mockExtState); }); + it('should throw when decrypting invalid message inline', async () => { + const messageToDecrypt = { + ...messageMock, + data: '0x7b2022666f6f223a202262617222207d', + }; + + expect( + decryptMessageController.decryptMessageInline(messageToDecrypt), + ).rejects.toThrow('Invalid encrypted data.'); + }); + it('should be able to cancel decrypt message', async () => { decryptMessageManagerMock.rejectMessage.mockResolvedValue(messageMock); getStateMock.mockReturnValue(mockExtState); diff --git a/app/scripts/controllers/decrypt-message.ts b/app/scripts/controllers/decrypt-message.ts index a0d1808063f1..1ace629dd995 100644 --- a/app/scripts/controllers/decrypt-message.ts +++ b/app/scripts/controllers/decrypt-message.ts @@ -5,7 +5,6 @@ import { DecryptMessageParams, DecryptMessageParamsMetamask, } from '@metamask/message-manager'; -import { KeyringController } from '@metamask/eth-keyring-controller'; import { AbstractMessage, AbstractMessageManager, @@ -25,6 +24,8 @@ import { } from '@metamask/approval-controller'; import { ApprovalType, ORIGIN_METAMASK } from '@metamask/controller-utils'; import { Patch } from 'immer'; +import type { KeyringControllerDecryptMessageAction } from '@metamask/keyring-controller'; +import { Eip1024EncryptedData, hasProperty, isObject } from '@metamask/utils'; import { MetaMetricsEventCategory } from '../../../shared/constants/metametrics'; import { stripHexPrefix } from '../../../shared/modules/hexstring-utils'; @@ -35,6 +36,37 @@ const stateMetadata = { unapprovedDecryptMsgCount: { persist: false, anonymous: false }, }; +/** + * Type guard that checks for the presence of the required properties + * for EIP-1024 encrypted data. + * + * See: https://eips.ethereum.org/EIPS/eip-1024 + * + * @param message - The message to check. + * @param message.from - The sender of the message. + * @param message.data - The EIP-1024 encrypted data. + * @returns Whether the message is an EIP-1024 encrypted message. + */ +export const isEIP1024EncryptedMessage = (message: { + from: string; + data: unknown; +}): message is { + from: string; + data: Eip1024EncryptedData; +} => { + if ( + isObject(message.data) && + hasProperty(message.data, 'version') && + hasProperty(message.data, 'nonce') && + hasProperty(message.data, 'ephemPublicKey') && + hasProperty(message.data, 'ciphertext') + ) { + return true; + } + + return false; +}; + export const getDefaultState = () => ({ unapprovedDecryptMsgs: {}, unapprovedDecryptMsgCount: 0, @@ -70,7 +102,11 @@ export type DecryptMessageControllerActions = GetDecryptMessageState; export type DecryptMessageControllerEvents = DecryptMessageStateChange; -type AllowedActions = AddApprovalRequest | AcceptRequest | RejectRequest; +type AllowedActions = + | AddApprovalRequest + | AcceptRequest + | RejectRequest + | KeyringControllerDecryptMessageAction; export type DecryptMessageControllerMessenger = RestrictedControllerMessenger< typeof controllerName, @@ -82,7 +118,6 @@ export type DecryptMessageControllerMessenger = RestrictedControllerMessenger< export type DecryptMessageControllerOptions = { getState: () => any; - keyringController: KeyringController; messenger: DecryptMessageControllerMessenger; metricsEvent: (payload: any, options?: any) => void; }; @@ -99,8 +134,6 @@ export default class DecryptMessageController extends BaseControllerV2< private _getState: () => any; - private _keyringController: KeyringController; - private _metricsEvent: (payload: any, options?: any) => void; private _decryptMessageManager: DecryptMessageManager; @@ -110,13 +143,11 @@ export default class DecryptMessageController extends BaseControllerV2< * * @param options - The controller options. * @param options.getState - Callback to retrieve all user state. - * @param options.keyringController - An instance of a keyring controller used to decrypt message * @param options.messenger - A reference to the messaging system. * @param options.metricsEvent - A function for emitting a metric event. */ constructor({ getState, - keyringController, metricsEvent, messenger, }: DecryptMessageControllerOptions) { @@ -127,7 +158,6 @@ export default class DecryptMessageController extends BaseControllerV2< state: getDefaultState(), }); this._getState = getState; - this._keyringController = keyringController; this._metricsEvent = metricsEvent; this.hub = new EventEmitter(); @@ -216,7 +246,12 @@ export default class DecryptMessageController extends BaseControllerV2< await this._decryptMessageManager.approveMessage(messageParams); cleanMessageParams.data = this._parseMessageData(cleanMessageParams.data); - const rawMessage = await this._keyringController.decryptMessage( + if (!isEIP1024EncryptedMessage(cleanMessageParams)) { + throw new Error('Invalid encrypted data.'); + } + + const rawMessage = await this.messagingSystem.call( + 'KeyringController:decryptMessage', cleanMessageParams, ); @@ -243,7 +278,11 @@ export default class DecryptMessageController extends BaseControllerV2< async decryptMessageInline(messageParams: DecryptMessageParamsMetamask) { const messageId = messageParams.metamaskId as string; messageParams.data = this._parseMessageData(messageParams.data); - const rawMessage = await this._keyringController.decryptMessage( + if (!isEIP1024EncryptedMessage(messageParams)) { + throw new Error('Invalid encrypted data.'); + } + const rawMessage = await this.messagingSystem.call( + 'KeyringController:decryptMessage', messageParams, ); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a68676d55ebe..42f6fd9a19e2 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1373,13 +1373,13 @@ export default class MetamaskController extends EventEmitter { this.networkController.lookupNetwork(); this.decryptMessageController = new DecryptMessageController({ getState: this.getState.bind(this), - keyringController: this.keyringController, messenger: this.controllerMessenger.getRestricted({ name: 'DecryptMessageController', allowedActions: [ `${this.approvalController.name}:addRequest`, `${this.approvalController.name}:acceptRequest`, `${this.approvalController.name}:rejectRequest`, + `${this.coreKeyringController.name}:decryptMessage`, ], }), metricsEvent: this.metaMetricsController.trackEvent.bind( diff --git a/package.json b/package.json index 66a2804d8f03..8e46c5c8119f 100644 --- a/package.json +++ b/package.json @@ -202,7 +202,7 @@ "request@^2.88.2": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch", "request@^2.85.0": "patch:request@npm%3A2.88.2#./.yarn/patches/request-npm-2.88.2-f4a57c72c4.patch", "lavamoat-core@^14.2.0": "patch:lavamoat-core@npm%3A14.2.0#./.yarn/patches/lavamoat-core-npm-14.2.0-c453f4f755.patch", - "@metamask/keyring-controller@^7.2.0": "patch:@metamask/keyring-controller@npm%3A7.2.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch", + "@metamask/keyring-controller@^7.4.0": "patch:@metamask/keyring-controller@npm%3A7.4.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch", "@metamask/signature-controller@^5.3.0": "patch:@metamask/signature-controller@npm%3A5.3.0#./.yarn/patches/@metamask-signature-controller-npm-5.3.0-225628460b.patch", "semver@7.3.7": "^7.5.4", "semver@7.3.8": "^7.5.4" @@ -252,7 +252,7 @@ "@metamask/gas-fee-controller": "^6.0.1", "@metamask/jazzicon": "^2.0.0", "@metamask/key-tree": "^9.0.0", - "@metamask/keyring-controller": "^7.2.0", + "@metamask/keyring-controller": "^7.4.0", "@metamask/logo": "^3.1.1", "@metamask/message-manager": "^7.3.0", "@metamask/metamask-eth-abis": "^3.0.0", diff --git a/yarn.lock b/yarn.lock index 371021672510..6781c948eca6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4285,9 +4285,9 @@ __metadata: languageName: node linkType: hard -"@metamask/keyring-controller@npm:7.2.0": - version: 7.2.0 - resolution: "@metamask/keyring-controller@npm:7.2.0" +"@metamask/keyring-controller@npm:7.4.0": + version: 7.4.0 + resolution: "@metamask/keyring-controller@npm:7.4.0" dependencies: "@keystonehq/metamask-airgapped-keyring": "npm:^0.13.1" "@metamask/base-controller": "npm:^3.2.1" @@ -4301,13 +4301,13 @@ __metadata: immer: "npm:^9.0.6" peerDependencies: "@metamask/preferences-controller": ^4.4.0 - checksum: 3535d698f68f732658787943eaff3ebabdc1e3d0e6bce731cff413b0fb98d19fb127841e525d9031fa96b50e75840686da9cd0615ab6772914e2b18fe1797df1 + checksum: 17b3f24d57dc79689ee6c0159f931ba71b10cbb728cecce6595ec69c46bc706289e90e22b9e7709fef8719ff99594adccd842a3a687fcc651a55cd873f2bc219 languageName: node linkType: hard -"@metamask/keyring-controller@patch:@metamask/keyring-controller@npm%3A7.2.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch": - version: 7.2.0 - resolution: "@metamask/keyring-controller@patch:@metamask/keyring-controller@npm%3A7.2.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.2.0-fcc0c7893b.patch::version=7.2.0&hash=6f51dc" +"@metamask/keyring-controller@patch:@metamask/keyring-controller@npm%3A7.4.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch": + version: 7.4.0 + resolution: "@metamask/keyring-controller@patch:@metamask/keyring-controller@npm%3A7.4.0#~/.yarn/patches/@metamask-keyring-controller-npm-7.4.0-5dd5df31c7.patch::version=7.4.0&hash=4a7ca3" dependencies: "@keystonehq/metamask-airgapped-keyring": "npm:^0.13.1" "@metamask/base-controller": "npm:^3.2.1" @@ -4321,7 +4321,7 @@ __metadata: immer: "npm:^9.0.6" peerDependencies: "@metamask/preferences-controller": ^4.4.0 - checksum: b3f0ffac12077c19bef8d978237b24ebe40c2bbee1363ee5cfdf7a09b600540981d3bdbac96845645cd976e0bdb55f56f80442a2797e1538abb00744194edf17 + checksum: d6965c9e3f7fe00481ed544fae172aa04f8212e570ccea4c1d16f52f42c775759df59a1009c52937e1f6c6a764c16b80b89c457a4765009ff9bb1d454aab42ab languageName: node linkType: hard @@ -24221,7 +24221,7 @@ __metadata: "@metamask/gas-fee-controller": "npm:^6.0.1" "@metamask/jazzicon": "npm:^2.0.0" "@metamask/key-tree": "npm:^9.0.0" - "@metamask/keyring-controller": "npm:^7.2.0" + "@metamask/keyring-controller": "npm:^7.4.0" "@metamask/logo": "npm:^3.1.1" "@metamask/message-manager": "npm:^7.3.0" "@metamask/metamask-eth-abis": "npm:^3.0.0"