fix: Sanitize signTypedDatav3v4 params before calling security API (#…

…29343)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR aims to filter request params before calling security API call
if method is `signTypedDatav3v4`

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29343?quickstart=1)

## **Related issues**

Fixes: MetaMask/MetaMask-planning#3830

## **Manual testing steps**

1. Copy the following payload 

```
// Request the current account addresses from the Ethereum provider
const addresses = await window.ethereum.request({ "method": "eth_accounts" });

// Construct the JSON string for eth_signTypedData_v4, including the dynamic owner address
const jsonData = {
  domain: {
    name: "USD Coin",
    version: "2",
    chainId: "1",
    verifyingContract: "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48"
  },
  types: {
    EIP712Domain: [
      { name: "name", type: "string" },
      { name: "version", type: "string" },
      { name: "chainId", type: "uint256" },
      { name: "verifyingContract", type: "address" }
    ],
    Permit: [
      { name: "owner", type: "address" },
      { name: "spender", type: "address" },
      { name: "value", type: "uint256" },
      { name: "nonce", type: "uint256" },
      { name: "deadline", type: "uint256" }
    ]
  },
  primaryType: "Permit",
  message: {
    owner: addresses[0],
    spender: "0xa2d86c5ff6fbf5f455b1ba2737938776c24d7a58",
    value: "115792089237316195423570985008687907853269984665640564039457584007913129639935",
    nonce: "0",
    deadline: "115792089237316195423570985008687907853269984665640564039457584007913129639935"
  }
};

// Use the first account address for signing the typed data
window.ethereum.sendAsync({
  method: "eth_signTypedData_v4",
  params: [
    addresses[0],
    JSON.stringify(jsonData),
    {},
    {},
    {}
  ]
});
```
2. Navigate to MM E2E Test Dapp > Connect Wallet > Open up the console >
Paste the payload above > Hit enter
3. Notice that the transaction is considered as malicious (which was not
flagged before)

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->



https://github.com/user-attachments/assets/ffcdd83f-bb79-4490-b729-f96559ce5769



### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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.

app/scripts/lib/ppom/ppom-util.test.ts

@@ -10,7 +10,7 @@ import {
   SignatureController,
   SignatureRequest,
 } from '@metamask/signature-controller';
-import { Hex } from '@metamask/utils';
+import { Hex, JsonRpcRequest } from '@metamask/utils';
 import {
   BlockaidReason,
   BlockaidResultType,
@@ -22,6 +22,8 @@ import { AppStateController } from '../../controllers/app-state-controller';
 import {
   generateSecurityAlertId,
   isChainSupported,
+  METHOD_SIGN_TYPED_DATA_V3,
+  METHOD_SIGN_TYPED_DATA_V4,
   updateSecurityAlertResponse,
   validateRequestWithPPOM,
 } from './ppom-util';
@@ -57,6 +59,10 @@ const TRANSACTION_PARAMS_MOCK_1: TransactionParams = {
   value: '0x123',
 };
 
+const SIGN_TYPED_DATA_PARAMS_MOCK_1 = '0x123';
+const SIGN_TYPED_DATA_PARAMS_MOCK_2 =
+  '{"primaryType":"Permit","domain":{},"types":{}}';
+
 const TRANSACTION_PARAMS_MOCK_2: TransactionParams = {
   ...TRANSACTION_PARAMS_MOCK_1,
   to: '0x456',
@@ -261,6 +267,48 @@ describe('PPOM Utils', () => {
       );
     });
 
+    // @ts-expect-error This is missing from the Mocha type definitions
+    it.each([METHOD_SIGN_TYPED_DATA_V3, METHOD_SIGN_TYPED_DATA_V4])(
+      'sanitizes request params if method is %s',
+      async (method: string) => {
+        const ppom = createPPOMMock();
+        const ppomController = createPPOMControllerMock();
+
+        ppomController.usePPOM.mockImplementation(
+          (callback) =>
+            // eslint-disable-next-line @typescript-eslint/no-explicit-any
+            callback(ppom as any) as any,
+        );
+
+        const firstTwoParams = [
+          SIGN_TYPED_DATA_PARAMS_MOCK_1,
+          SIGN_TYPED_DATA_PARAMS_MOCK_2,
+        ];
+
+        const unwantedParams = [{}, undefined, 1, null];
+
+        const params = [...firstTwoParams, ...unwantedParams];
+
+        const request = {
+          ...REQUEST_MOCK,
+          method,
+          params,
+        } as unknown as JsonRpcRequest;
+
+        await validateRequestWithPPOM({
+          ...validateRequestWithPPOMOptionsBase,
+          ppomController,
+          request,
+        });
+
+        expect(ppom.validateJsonRpc).toHaveBeenCalledTimes(1);
+        expect(ppom.validateJsonRpc).toHaveBeenCalledWith({
+          ...request,
+          params: firstTwoParams,
+        });
+      },
+    );
+
     it('updates response indicating chain is not supported', async () => {
       const ppomController = {} as PPOMController;
       const CHAIN_ID_UNSUPPORTED_MOCK = '0x2';

app/scripts/lib/ppom/ppom-util.ts

@@ -29,6 +29,8 @@ import {
 const { sentry } = global;
 
 const METHOD_SEND_TRANSACTION = 'eth_sendTransaction';
+export const METHOD_SIGN_TYPED_DATA_V3 = 'eth_signTypedData_v3';
+export const METHOD_SIGN_TYPED_DATA_V4 = 'eth_signTypedData_v4';
 
 const SECURITY_ALERT_RESPONSE_ERROR = {
   result_type: BlockaidResultType.Errored,
@@ -171,7 +173,7 @@ function normalizePPOMRequest(
       request,
     )
   ) {
-    return request;
+    return sanitizeRequest(request);
   }
 
   const transactionParams = request.params[0];
@@ -183,6 +185,22 @@ function normalizePPOMRequest(
   };
 }
 
+function sanitizeRequest(request: JsonRpcRequest): JsonRpcRequest {
+  // This is a temporary fix to prevent a PPOM bypass
+  if (
+    request.method === METHOD_SIGN_TYPED_DATA_V4 ||
+    request.method === METHOD_SIGN_TYPED_DATA_V3
+  ) {
+    if (Array.isArray(request.params)) {
+      return {
+        ...request,
+        params: request.params.slice(0, 2),
+      };
+    }
+  }
+  return request;
+}
+
 function getErrorMessage(error: unknown) {
   if (error instanceof Error) {
     return `${error.name}: ${error.message}`;