feat: [WIP] Test #9274 + #9925

.iyarc

@@ -1,7 +1,3 @@
-# ReDoS vulnerability, no impact to this application, and fix not backported yet to the versions we use
-
-GHSA-c2qf-rxjj-qqgw
-
 # ip SSRF improper categorization in isPublic, since it only affect dev tools on, and the server is actually a local server, this advisory shouldn't apply to this use cases
 
 GHSA-2p57-rm9w-gvfp

app/core/BackgroundBridge/BackgroundBridge.js

@@ -1,7 +1,7 @@
 /* eslint-disable import/no-commonjs */
 import URL from 'url-parse';
 import { ChainId } from '@metamask/controller-utils';
-import { JsonRpcEngine } from 'json-rpc-engine';
+import { JsonRpcEngine } from '@metamask/json-rpc-engine';
 import MobilePortStream from '../MobilePortStream';
 import { setupMultiplex } from '../../util/streams';
 import {
@@ -22,14 +22,14 @@ import {
   selectProviderConfig,
 } from '../../selectors/networkController';
 import { store } from '../../store';
+const createFilterMiddleware = require('@metamask/eth-json-rpc-filters');
+const createSubscriptionManager = require('@metamask/eth-json-rpc-filters/subscriptionManager');
+import { providerAsMiddleware } from '@metamask/eth-json-rpc-middleware';
 ///: BEGIN:ONLY_INCLUDE_IF(snaps)
 import snapMethodMiddlewareBuilder from '../Snaps/SnapsMethodMiddleware';
 import { SubjectType } from '@metamask/permission-controller';
 ///: END:ONLY_INCLUDE_IF
 
-const createFilterMiddleware = require('eth-json-rpc-filters');
-const createSubscriptionManager = require('eth-json-rpc-filters/subscriptionManager');
-const providerAsMiddleware = require('eth-json-rpc-middleware/providerAsMiddleware');
 const pump = require('pump');
 // eslint-disable-next-line import/no-nodejs-modules
 const EventEmitter = require('events').EventEmitter;

app/core/RPCMethods/RPCMethodMiddleware.test.ts

@@ -1,14 +1,15 @@
-import {
-  JsonRpcEngine,
+import { JsonRpcEngine, JsonRpcMiddleware } from '@metamask/json-rpc-engine';
+import type { ProviderConfig } from '@metamask/network-controller';
+import { providerErrors, rpcErrors } from '@metamask/rpc-errors';
+import type { TransactionParams } from '@metamask/transaction-controller';
+import type {
+  Json,
   JsonRpcFailure,
-  JsonRpcMiddleware,
+  JsonRpcParams,
   JsonRpcRequest,
   JsonRpcResponse,
   JsonRpcSuccess,
-} from 'json-rpc-engine';
-import type { TransactionParams } from '@metamask/transaction-controller';
-import type { ProviderConfig } from '@metamask/network-controller';
-import { providerErrors, rpcErrors } from '@metamask/rpc-errors';
+} from '@metamask/utils';
 import Engine from '../Engine';
 import { store } from '../../store';
 import { getPermittedAccounts } from '../Permissions';
@@ -93,8 +94,8 @@ const jsonrpc = '2.0' as const;
  * @throws If the given value is not a valid {@link JsonRpcSuccess} object.
  */
 function assertIsJsonRpcSuccess(
-  response: JsonRpcResponse<unknown>,
-): asserts response is JsonRpcSuccess<unknown> {
+  response: JsonRpcResponse<Json>,
+): asserts response is JsonRpcSuccess<Json> {
   if ('error' in response) {
     throw new Error(`Response failed with error '${JSON.stringify('error')}'`);
   } else if (!('result' in response)) {
@@ -195,8 +196,8 @@ async function callMiddleware({
   middleware,
   request,
 }: {
-  middleware: JsonRpcMiddleware<unknown, unknown>;
-  request: JsonRpcRequest<unknown>;
+  middleware: JsonRpcMiddleware<JsonRpcParams, Json>;
+  request: JsonRpcRequest<JsonRpcParams>;
 }) {
   const engine = new JsonRpcEngine();
   engine.push(middleware);
@@ -376,7 +377,7 @@ describe('getRpcMethodMiddleware', () => {
       permissionController.createPermissionMiddleware({
         origin: hostMock,
       });
-    // @ts-expect-error JsonRpcId (number | string | void) doesn't match PS middleware's id, which is (string | number | null)
+    // @ts-expect-error TODO
     engine.push(permissionMiddleware);
     const middleware = getRpcMethodMiddleware(getMinimalOptions());
     engine.push(middleware);
@@ -943,7 +944,8 @@ describe('getRpcMethodMiddleware', () => {
     it('returns a JSON-RPC error if an error is thrown when adding this transaction', async () => {
       // Omit `from` and `chainId` here to skip validation for simplicity
       // Downcast needed here because `from` is required by this type
-      const mockTransactionParameters = {} as TransactionParams;
+      const mockTransactionParameters = {} as (TransactionParams &
+        JsonRpcParams)[];
       // Transaction fails before returning a result
       mockAddTransaction.mockImplementation(async () => {
         throw new Error('Failed to add transaction');
@@ -958,20 +960,26 @@ describe('getRpcMethodMiddleware', () => {
         method: 'eth_sendTransaction',
         params: [mockTransactionParameters],
       };
-      const expectedError = rpcErrors.internal('Failed to add transaction');
+      const expectedError = rpcErrors.internal('Internal JSON-RPC error.');
+      const expectedErrorCauseMessage = 'Failed to add transaction';
 
       const response = await callMiddleware({ middleware, request });
 
       expect((response as JsonRpcFailure).error.code).toBe(expectedError.code);
       expect((response as JsonRpcFailure).error.message).toBe(
         expectedError.message,
       );
+      // @ts-expect-error - TODO: This should type
+      expect((response as JsonRpcFailure).error.data.cause.message).toBe(
+        expectedErrorCauseMessage,
+      );
     });
 
     it('returns a JSON-RPC error if an error is thrown after approval', async () => {
       // Omit `from` and `chainId` here to skip validation for simplicity
       // Downcast needed here because `from` is required by this type
-      const mockTransactionParameters = {} as TransactionParams;
+      const mockTransactionParameters = {} as (TransactionParams &
+        JsonRpcParams)[];
       setupGlobalState({
         addTransactionResult: Promise.reject(
           new Error('Failed to process transaction'),
@@ -987,14 +995,19 @@ describe('getRpcMethodMiddleware', () => {
         method: 'eth_sendTransaction',
         params: [mockTransactionParameters],
       };
-      const expectedError = rpcErrors.internal('Failed to process transaction');
+      const expectedError = rpcErrors.internal('Internal JSON-RPC error.');
+      const expectedErrorCauseMessage = 'Failed to process transaction';
 
       const response = await callMiddleware({ middleware, request });
 
       expect((response as JsonRpcFailure).error.code).toBe(expectedError.code);
       expect((response as JsonRpcFailure).error.message).toBe(
         expectedError.message,
       );
+      // @ts-expect-error - TODO: This should type
+      expect((response as JsonRpcFailure).error.data.cause.message).toBe(
+        expectedErrorCauseMessage,
+      );
     });
   });
 
@@ -1060,7 +1073,7 @@ describe('getRpcMethodMiddleware', () => {
         method: 'personal_ecRecover',
         params: [helloWorldMessage],
       };
-      const expectedError = rpcErrors.internal('Missing signature parameter');
+      const expectedError = rpcErrors.internal('Internal JSON-RPC error.');
 
       const response = await callMiddleware({ middleware, request });
 
@@ -1079,9 +1092,9 @@ describe('getRpcMethodMiddleware', () => {
         jsonrpc,
         id: 1,
         method: 'personal_ecRecover',
-        params: [undefined, helloWorldSignature],
+        params: [undefined, helloWorldSignature] as JsonRpcParams,
       };
-      const expectedError = rpcErrors.internal('Missing data parameter');
+      const expectedError = rpcErrors.internal('Internal JSON-RPC error.');
 
       const response = await callMiddleware({ middleware, request });
 

app/core/RPCMethods/RPCMethodMiddleware.ts

@@ -1,6 +1,6 @@
 import { Alert } from 'react-native';
 import { getVersion } from 'react-native-device-info';
-import { createAsyncMiddleware } from 'json-rpc-engine';
+import { createAsyncMiddleware } from '@metamask/json-rpc-engine';
 import { providerErrors, rpcErrors } from '@metamask/rpc-errors';
 import {
   EndFlowOptions,
@@ -15,6 +15,7 @@ import {
   PermissionController,
   permissionRpcMethods,
 } from '@metamask/permission-controller';
+import type { Hex } from '@metamask/utils';
 import Networks, {
   blockTagParamIndex,
   getAllNetworks,
@@ -113,7 +114,7 @@ export const checkActiveAccountAndChainId = async ({
   isWalletConnect,
 }: {
   address?: string;
-  chainId?: number;
+  chainId?: Hex;
   channelId?: string;
   hostname: string;
   isWalletConnect: boolean;
@@ -224,7 +225,7 @@ const generateRawSignature = async ({
   title: { current: string };
   icon: { current: string | undefined };
   analytics: { [key: string]: string | boolean };
-  chainId: number;
+  chainId: Hex;
   isMMSDK: boolean;
   channelId?: string;
   getSource: () => string;
@@ -533,7 +534,7 @@ export const getRpcMethodMiddleware = ({
             chainId,
           }: {
             from?: string;
-            chainId?: number;
+            chainId?: Hex;
           }) => {
             await checkActiveAccountAndChainId({
               hostname,

app/core/RPCMethods/eth_sendTransaction.test.ts

@@ -1,6 +1,11 @@
 // eslint-disable-next-line import/no-nodejs-modules
 import { inspect } from 'util';
-import type { JsonRpcRequest, PendingJsonRpcResponse } from 'json-rpc-engine';
+import type {
+  Json,
+  JsonRpcParams,
+  JsonRpcRequest,
+  PendingJsonRpcResponse,
+} from '@metamask/utils';
 import type {
   TransactionParams,
   TransactionController,
@@ -39,14 +44,16 @@ jest.mock('../../util/transaction-controller', () => ({
  * @param params - The request parameters.
  * @returns The JSON-RPC request.
  */
-function constructSendTransactionRequest(
-  params: unknown,
-): JsonRpcRequest<unknown> & { method: 'eth_sendTransaction' } {
+function constructSendTransactionRequest(params: Json[]): JsonRpcRequest<
+  [TransactionParams & JsonRpcParams]
+> & {
+  method: 'eth_sendTransaction';
+} {
   return {
     jsonrpc: '2.0',
     id: 1,
     method: 'eth_sendTransaction',
-    params,
+    params: params as any,
   };
 }
 
@@ -55,7 +62,7 @@ function constructSendTransactionRequest(
  *
  * @returns A pending JSON-RPC response.
  */
-function constructPendingJsonRpcResponse(): PendingJsonRpcResponse<unknown> {
+function constructPendingJsonRpcResponse(): PendingJsonRpcResponse<Json> {
   return {
     jsonrpc: '2.0',
     id: 1,
@@ -136,13 +143,17 @@ function getMockAddTransaction({
 describe('eth_sendTransaction', () => {
   it('sends the transaction and returns the resulting hash', async () => {
     const mockAddress = '0x0000000000000000000000000000000000000001';
-    const mockTransactionParameters = { from: mockAddress };
+    const mockTransactionParameters = {
+      from: mockAddress,
+    } as TransactionParams;
     const expectedResult = 'fake-hash';
     const pendingResult = constructPendingJsonRpcResponse();
 
     await eth_sendTransaction({
       hostname: 'example.metamask.io',
-      req: constructSendTransactionRequest([mockTransactionParameters]),
+      req: constructSendTransactionRequest([
+        mockTransactionParameters as unknown as JsonRpcParams,
+      ]) as any,
       res: pendingResult,
       sendTransaction: getMockAddTransaction({
         expectedTransaction: mockTransactionParameters,
@@ -164,7 +175,9 @@ describe('eth_sendTransaction', () => {
         async () =>
           await eth_sendTransaction({
             hostname: 'example.metamask.io',
-            req: constructSendTransactionRequest(invalidParameter),
+            req: constructSendTransactionRequest(
+              invalidParameter as unknown as Json[],
+            ),
             res: constructPendingJsonRpcResponse(),
             sendTransaction: getMockAddTransaction({
               returnValue: 'fake-hash',
@@ -186,7 +199,9 @@ describe('eth_sendTransaction', () => {
         async () =>
           await eth_sendTransaction({
             hostname: 'example.metamask.io',
-            req: constructSendTransactionRequest(invalidParameter),
+            req: constructSendTransactionRequest(
+              invalidParameter as unknown as Json[],
+            ),
             res: constructPendingJsonRpcResponse(),
             sendTransaction: getMockAddTransaction({
               returnValue: 'fake-hash',

app/core/RPCMethods/eth_sendTransaction.ts

@@ -1,6 +1,13 @@
-import type { JsonRpcRequest, PendingJsonRpcResponse } from 'json-rpc-engine';
+import type {
+  Hex,
+  Json,
+  JsonRpcParams,
+  JsonRpcRequest,
+  PendingJsonRpcResponse,
+} from '@metamask/utils';
 import {
   TransactionController,
+  TransactionParams,
   WalletDevice,
 } from '@metamask/transaction-controller';
 import { rpcErrors } from '@metamask/rpc-errors';
@@ -46,6 +53,11 @@ const hasProperty = <
 ): objectToCheck is ObjectToCheck & Record<Property, unknown> =>
   Object.hasOwnProperty.call(objectToCheck, name);
 
+interface SendArgs {
+  from: string;
+  chainId?: Hex;
+}
+
 /**
  * Handle a `eth_sendTransaction` request.
  *
@@ -66,13 +78,12 @@ async function eth_sendTransaction({
   validateAccountAndChainId,
 }: {
   hostname: string;
-  req: JsonRpcRequest<unknown> & { method: 'eth_sendTransaction' };
-  res: PendingJsonRpcResponse<unknown>;
+  req: JsonRpcRequest<[TransactionParams & JsonRpcParams]> & {
+    method: 'eth_sendTransaction';
+  };
+  res: PendingJsonRpcResponse<Json>;
   sendTransaction: TransactionController['addTransaction'];
-  validateAccountAndChainId: (args: {
-    from: string;
-    chainId?: number;
-  }) => Promise<void>;
+  validateAccountAndChainId: (args: SendArgs) => Promise<void>;
 }) {
   if (
     !Array.isArray(req.params) &&

app/core/RPCMethods/wallet_watchAsset.ts

@@ -11,7 +11,7 @@ import {
 } from '../../constants/error';
 import { selectChainId } from '../../selectors/networkController';
 import { isValidAddress } from 'ethereumjs-util';
-import { JsonRpcRequest, PendingJsonRpcResponse } from 'json-rpc-engine';
+import { JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils';
 
 const wallet_watchAsset = async ({
   req,