fix: validate EIP-5792 requests using PPOM (#31231)

matthewwalsh0 · web-flow · commit 7acea0ef2a95 · 2025-03-26T12:00:46.000Z
## **Description** Validate security for all EIP-5792 requests. Specifically: - Use the new `validateSecurity` callback in the `addTransactionBatch` controller method. - Bump `@metamask/transaction-controller` to `^52.2.0`. - Fix E2E tests due to version change. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31231?quickstart=1) ## **Related issues** Fixes: #31263 ## **Manual testing steps** See issue. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
diff --git a/app/scripts/lib/transaction/eip5792.test.ts b/app/scripts/lib/transaction/eip5792.test.ts
@@ -86,6 +86,10 @@ describe('EIP-5792', () => {
 
   let getDisabledAccountUpgradeChainsMock: jest.MockedFn<() => Hex[]>;
 
+  let validateSecurityMock: jest.MockedFunction<
+    Parameters<typeof processSendCalls>[0]['validateSecurity']
+  >;
+
   let messenger: EIP5792Messenger;
 
   beforeEach(() => {
@@ -95,6 +99,7 @@ describe('EIP-5792', () => {
     getTransactionControllerStateMock = jest.fn();
     getNetworkClientByIdMock = jest.fn();
     getDisabledAccountUpgradeChainsMock = jest.fn();
+    validateSecurityMock = jest.fn();
 
     messenger = new Messenger();
 
@@ -117,6 +122,7 @@ describe('EIP-5792', () => {
     addTransactionBatchMock.mockResolvedValue({
       batchId: BATCH_ID_MOCK,
     });
+
     getDisabledAccountUpgradeChainsMock.mockReturnValue([]);
   });
 
@@ -126,6 +132,7 @@ describe('EIP-5792', () => {
         {
           addTransactionBatch: addTransactionBatchMock,
           getDisabledAccountUpgradeChains: getDisabledAccountUpgradeChainsMock,
+          validateSecurity: validateSecurityMock,
         },
         messenger,
         SEND_CALLS_MOCK,
@@ -136,7 +143,9 @@ describe('EIP-5792', () => {
         from: SEND_CALLS_MOCK.from,
         networkClientId: NETWORK_CLIENT_ID_MOCK,
         origin: ORIGIN_MOCK,
+        securityAlertId: expect.any(String),
         transactions: [{ params: SEND_CALLS_MOCK.calls[0] }],
+        validateSecurity: expect.any(Function),
       });
     });
 
@@ -147,6 +156,7 @@ describe('EIP-5792', () => {
             addTransactionBatch: addTransactionBatchMock,
             getDisabledAccountUpgradeChains:
               getDisabledAccountUpgradeChainsMock,
+            validateSecurity: validateSecurityMock,
           },
           messenger,
           SEND_CALLS_MOCK,
@@ -162,6 +172,7 @@ describe('EIP-5792', () => {
             addTransactionBatch: addTransactionBatchMock,
             getDisabledAccountUpgradeChains:
               getDisabledAccountUpgradeChainsMock,
+            validateSecurity: validateSecurityMock,
           },
           messenger,
           { ...SEND_CALLS_MOCK, version: '2.0' },
@@ -177,6 +188,7 @@ describe('EIP-5792', () => {
             addTransactionBatch: addTransactionBatchMock,
             getDisabledAccountUpgradeChains:
               getDisabledAccountUpgradeChainsMock,
+            validateSecurity: validateSecurityMock,
           },
           messenger,
           { ...SEND_CALLS_MOCK, chainId: CHAIN_ID_2_MOCK },
@@ -196,6 +208,7 @@ describe('EIP-5792', () => {
             addTransactionBatch: addTransactionBatchMock,
             getDisabledAccountUpgradeChains:
               getDisabledAccountUpgradeChainsMock,
+            validateSecurity: validateSecurityMock,
           },
           messenger,
           SEND_CALLS_MOCK,
@@ -213,6 +226,7 @@ describe('EIP-5792', () => {
             addTransactionBatch: addTransactionBatchMock,
             getDisabledAccountUpgradeChains:
               getDisabledAccountUpgradeChainsMock,
+            validateSecurity: validateSecurityMock,
           },
           messenger,
           {
@@ -235,6 +249,7 @@ describe('EIP-5792', () => {
             addTransactionBatch: addTransactionBatchMock,
             getDisabledAccountUpgradeChains:
               getDisabledAccountUpgradeChainsMock,
+            validateSecurity: validateSecurityMock,
           },
           messenger,
           {
diff --git a/app/scripts/lib/transaction/eip5792.ts b/app/scripts/lib/transaction/eip5792.ts
@@ -7,6 +7,7 @@ import {
   TransactionMeta,
   TransactionReceipt,
   TransactionStatus,
+  ValidateSecurityRequest,
 } from '@metamask/transaction-controller';
 import { Hex, JsonRpcRequest } from '@metamask/utils';
 import { Messenger } from '@metamask/base-controller';
@@ -16,6 +17,7 @@ import {
   SendCalls,
   SendCallsResult,
 } from '@metamask/eth-json-rpc-middleware';
+import { generateSecurityAlertId } from '../ppom/ppom-util';
 
 type Actions =
   | NetworkControllerGetNetworkClientByIdAction
@@ -30,12 +32,22 @@ export async function processSendCalls(
   hooks: {
     addTransactionBatch: TransactionController['addTransactionBatch'];
     getDisabledAccountUpgradeChains: () => Hex[];
+    validateSecurity: (
+      securityAlertId: string,
+      request: ValidateSecurityRequest,
+      chainId: Hex,
+    ) => Promise<void>;
   },
   messenger: EIP5792Messenger,
   params: SendCalls,
   req: JsonRpcRequest & { networkClientId: string; origin?: string },
 ): Promise<SendCallsResult> {
-  const { addTransactionBatch, getDisabledAccountUpgradeChains } = hooks;
+  const {
+    addTransactionBatch,
+    getDisabledAccountUpgradeChains,
+    validateSecurity: validateSecurityHook,
+  } = hooks;
+
   const { calls, from } = params;
   const { networkClientId, origin } = req;
   const transactions = calls.map((call) => ({ params: call }));
@@ -49,11 +61,16 @@ export async function processSendCalls(
 
   validateSendCalls(params, dappChainId, disabledChains);
 
+  const securityAlertId = generateSecurityAlertId();
+  const validateSecurity = validateSecurityHook.bind(null, securityAlertId);
+
   const { batchId: id } = await addTransactionBatch({
     from,
     networkClientId,
     origin,
+    securityAlertId,
     transactions,
+    validateSecurity,
   });
 
   return { id };
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
@@ -337,7 +337,10 @@ import { METAMASK_COOKIE_HANDLER } from './constants/stream';
 
 // Notification controllers
 import { createTxVerificationMiddleware } from './lib/tx-verification/tx-verification-middleware';
-import { updateSecurityAlertResponse } from './lib/ppom/ppom-util';
+import {
+  updateSecurityAlertResponse,
+  validateRequestWithPPOM,
+} from './lib/ppom/ppom-util';
 import createEvmMethodsToNonEvmAccountReqFilterMiddleware from './lib/createEvmMethodsToNonEvmAccountReqFilterMiddleware';
 import { isEthAddress } from './lib/multichain/address';
 
@@ -2089,6 +2092,15 @@ export default class MetamaskController extends EventEmitter {
             this.preferencesController.getDisabledAccountUpgradeChains.bind(
               this.preferencesController,
             ),
+          validateSecurity: (securityAlertId, request, chainId) =>
+            validateRequestWithPPOM({
+              chainId,
+              ppomController: this.ppomController,
+              request,
+              securityAlertId,
+              updateSecurityAlertResponse:
+                this.updateSecurityAlertResponse.bind(this),
+            }),
         },
         this.controllerMessenger,
       ),
diff --git a/package.json b/package.json
@@ -325,7 +325,7 @@
     "@metamask/snaps-sdk": "^6.19.0",
     "@metamask/snaps-utils": "^9.0.1",
     "@metamask/solana-wallet-snap": "^1.15.1",
-    "@metamask/transaction-controller": "^52.1.0",
+    "@metamask/transaction-controller": "^52.2.0",
     "@metamask/user-operation-controller": "^24.0.1",
     "@metamask/utils": "^11.1.0",
     "@ngraveio/bc-ur": "^1.1.12",
diff --git a/test/e2e/tests/simulation-details/mock-request-buy-erc1155.ts b/test/e2e/tests/simulation-details/mock-request-buy-erc1155.ts
@@ -7,8 +7,6 @@ import {
 export const BUY_ERC1155_TRANSACTION_MOCK = {
   data: '0xe7acab24000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000006200000007b02230091a7ed01230072f7006a004d60a8d4e71d599b8104250f0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000005200000000000000000000000000000000000000000000000000000000000000580000000000000000000000000a0d884d99316d14d7727a6a9223a8266a4595468000000000000000000000000004c00500000ad104d7dbd00e3ae0a5c00560c000000000000000000000000000000000000000000000000000000000000000160000000000000000000000000000000000000000000000000000000000000022000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000065e517b200000000000000000000000000000000000000000000000000000000660de8220000000000000000000000000000000000000000000000000000000000000000360c6ebe0000000000000000000000000000000000000000187d7f63de4580450000007b02230091a7ed01230072f7006a004d60a8d4e71d599b8104250f000000000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000300000000000000000000000076be3b62873462d2142405439777e971754e8e77000000000000000000000000000000000000000000000000000000000000286400000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000059874438570000000000000000000000000000000000000000000000000000005987443857000000000000000000000000000a0d884d99316d14d7727a6a9223a8266a4595468000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000028ed6103d000000000000000000000000000000000000000000000000000000028ed6103d0000000000000000000000000000000a26b00c1f0df003000390027140000faa7190000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000a3b5840f40000000000000000000000000000000000000000000000000000000a3b5840f4000000000000000000000000000157e23d3e68ac6f99334b8b0fe71f0eb844911dd0000000000000000000000000000000000000000000000000000000000000040100fc5b608d71976e87680aa0b785ae11ac8b9004316b9721222a5d6bea2c7a346d74d85776450c82d2ee3e4fa24eb8d612f5975a577789ad2b58dfc6a6e49760000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000360c6ebe',
   from: SENDER_ADDRESS_MOCK,
-  maxFeePerGas: '0x0',
-  maxPriorityFeePerGas: '0x0',
   to: '0x00000000000000adc04c56bf30ac9d3c0aaf14dc',
   value: '0x19945ca262000',
 };
diff --git a/test/e2e/tests/simulation-details/mock-request-buy-erc20.ts b/test/e2e/tests/simulation-details/mock-request-buy-erc20.ts
@@ -7,8 +7,6 @@ import {
 export const BUY_ERC20_TRANSACTION = {
   data: `0x5ae401dc0000000000000000000000000000000000000000000000000000000065fb43ab0000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000124b858183f00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000080000000000000000000000000${SENDER_ADDRESS_NO_0X_MOCK}00000000000000000000000000000000000000000000000000071afd498d00000000000000000000000000000000000000000000000000005d4bc0025786653d0000000000000000000000000000000000000000000000000000000000000042c02aaa39b223fe8d0a0e5c4f27ead9083c756cc20001f4a0b86991c6218b36c1d19d4a2e9eb0ce3606eb480000646b175474e89094c44da98b954eedeac495271d0f00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000`,
   from: SENDER_ADDRESS_MOCK,
-  maxFeePerGas: '0x0',
-  maxPriorityFeePerGas: '0x0',
   to: '0x13f4ea83d0bd40e75c8222255bc855a974568dd4',
   value: '0x71afd498d0000',
 };
diff --git a/test/e2e/tests/simulation-details/mock-request-buy-erc721.ts b/test/e2e/tests/simulation-details/mock-request-buy-erc721.ts
@@ -7,8 +7,6 @@ import {
 export const BUY_ERC721_TRANSACTION_MOCK = {
   data: '0xfbee349d00000000000000000000000000000000000000000000000000000000000000c00000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000001ccc7b6351d59f42bbb85f1fcc89cd4ac31c69cfa0681c900c46ddb7ad3f3c690f1006752ecedda5ee83902884f78ae85a3a1996bc56dc30f6aba0004ddc87699f00000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ff900d7067a24874264ced8198cc87ebd7c1c53d0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006669f99b0000000000000000000000000000000024081bbd5ce9458a998286f53a873a41000000000000000000000000eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee000000000000000000000000000000000000000000000000002e01f4d5d760000000000000000000000000000000000000000000000000000000000000000160000000000000000000000000ef9c21e3ba31a74910fc7e7cb3fc814ad842ad6e00000000000000000000000000000000000000000000000000000000000002cf000000000000000000000000000000000000000000000000000000000000022000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000020000000000000000000000000e9ba8fa0222fc76e8bcaa13e4258f749bb92af320000000000000000000000000000000000000000000000000003baf82d03a0000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000',
   from: SENDER_ADDRESS_MOCK,
-  maxFeePerGas: '0x0',
-  maxPriorityFeePerGas: '0x0',
   to: '0xdef1c0ded9bec7f1a1670819833240f027b25eff',
   value: '0x31bced02db0000',
 };
diff --git a/test/e2e/tests/simulation-details/mock-request-error-insuffient-gas.ts b/test/e2e/tests/simulation-details/mock-request-error-insuffient-gas.ts
@@ -6,8 +6,6 @@ import {
 
 export const INSUFFICIENT_GAS_TRANSACTION_MOCK = {
   from: SENDER_ADDRESS_MOCK,
-  maxFeePerGas: '0x0',
-  maxPriorityFeePerGas: '0x0',
   to: RECIPIENT_ADDRESS_MOCK,
   value: '0x6BC75E2D63100000',
 };
diff --git a/test/e2e/tests/simulation-details/mock-request-error-malformed-transaction.ts b/test/e2e/tests/simulation-details/mock-request-error-malformed-transaction.ts
@@ -3,8 +3,6 @@ import { MockRequestResponse, SENDER_ADDRESS_MOCK } from './types';
 export const MALFORMED_TRANSACTION_MOCK = {
   data: '0x0323498273498729872340897234087q',
   from: SENDER_ADDRESS_MOCK,
-  maxFeePerGas: '0x0',
-  maxPriorityFeePerGas: '0x0',
   to: '0xe18035bf8712672935fdb4e5e431b1a0183d2dfc',
   value: '0x0',
 };
diff --git a/test/e2e/tests/simulation-details/mock-request-no-changes.ts b/test/e2e/tests/simulation-details/mock-request-no-changes.ts
@@ -2,8 +2,6 @@ import { MockRequestResponse, SENDER_ADDRESS_MOCK } from './types';
 
 export const NO_CHANGES_TRANSACTION_MOCK = {
   from: SENDER_ADDRESS_MOCK,
-  maxFeePerGas: '0x0',
-  maxPriorityFeePerGas: '0x0',
   to: SENDER_ADDRESS_MOCK,
   value: '0x0',
 };
diff --git a/test/e2e/tests/simulation-details/mock-request-send-eth.ts b/test/e2e/tests/simulation-details/mock-request-send-eth.ts
@@ -7,8 +7,6 @@ import {
 export const SEND_ETH_TRANSACTION_MOCK = {
   data: '0x',
   from: SENDER_ADDRESS_MOCK,
-  maxFeePerGas: '0x0',
-  maxPriorityFeePerGas: '0x0',
   to: RECIPIENT_ADDRESS_MOCK,
   value: '0x38d7ea4c68000',
 };
diff --git a/yarn.lock b/yarn.lock
@@ -6532,9 +6532,9 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@metamask/transaction-controller@npm:^52.1.0":
-  version: 52.1.0
-  resolution: "@metamask/transaction-controller@npm:52.1.0"
+"@metamask/transaction-controller@npm:^52.2.0":
+  version: 52.2.0
+  resolution: "@metamask/transaction-controller@npm:52.2.0"
   dependencies:
     "@ethereumjs/common": "npm:^4.4.0"
     "@ethereumjs/tx": "npm:^5.4.0"
@@ -6564,7 +6564,7 @@ __metadata:
     "@metamask/gas-fee-controller": ^23.0.0
     "@metamask/network-controller": ^23.0.0
     "@metamask/remote-feature-flag-controller": ^1.5.0
-  checksum: 10/934d90160ec2094a5f0f9f1c8a9a38580522744b921c94ec4c2b59d3459fb7ecfcde18c21ba0d3b113b428ac3d812c9d123c78868e679d03bda6c3c971ca9415
+  checksum: 10/2a2b08c5d19410a69cf3fd29819d7928e91c3d9fe59b1b29a12ff62ddebacadcb3dc4710e54860571230f95ed17c57fc6876d85fcfbc6d6b21e22d868a47cb92
   languageName: node
   linkType: hard
 
@@ -27339,7 +27339,7 @@ __metadata:
     "@metamask/test-bundler": "npm:^1.0.0"
     "@metamask/test-dapp": "npm:9.2.0"
     "@metamask/test-dapp-multichain": "npm:^0.6.0"
-    "@metamask/transaction-controller": "npm:^52.1.0"
+    "@metamask/transaction-controller": "npm:^52.2.0"
     "@metamask/user-operation-controller": "npm:^24.0.1"
     "@metamask/utils": "npm:^11.1.0"
     "@ngraveio/bc-ur": "npm:^1.1.12"
