diff --git a/app/lib/ppom/ppom-util.test.ts b/app/lib/ppom/ppom-util.test.ts index 89faa23a585..6d0ab6bc03d 100644 --- a/app/lib/ppom/ppom-util.test.ts +++ b/app/lib/ppom/ppom-util.test.ts @@ -3,7 +3,10 @@ import * as SignatureRequestActions from '../../actions/signatureRequest'; // es import * as TransactionActions from '../../actions/transaction'; // eslint-disable-line import/no-namespace import * as NetworkControllerSelectors from '../../selectors/networkController'; // eslint-disable-line import/no-namespace import Engine from '../../core/Engine'; -import PPOMUtil from './ppom-util'; +import PPOMUtil, { + METHOD_SIGN_TYPED_DATA_V3, + METHOD_SIGN_TYPED_DATA_V4, +} from './ppom-util'; // eslint-disable-next-line import/no-namespace import * as securityAlertAPI from './security-alerts-api'; import { isBlockaidFeatureEnabled } from '../../util/blockaid'; @@ -22,6 +25,10 @@ import Logger from '../../util/Logger'; const CHAIN_ID_MOCK = '0x1'; +const SIGN_TYPED_DATA_PARAMS_MOCK_1 = '0x123'; +const SIGN_TYPED_DATA_PARAMS_MOCK_2 = + '{"primaryType":"Permit","domain":{},"types":{}}'; + jest.mock('./security-alerts-api'); jest.mock('../../util/blockaid'); @@ -439,5 +446,38 @@ describe('PPOM Utils', () => { source: SecurityAlertSource.Local, }); }); + + it.each([METHOD_SIGN_TYPED_DATA_V3, METHOD_SIGN_TYPED_DATA_V4])( + 'sanitizes request params if method is %s', + async (method: string) => { + isSecurityAlertsEnabledMock.mockReturnValue(true); + getSupportedChainIdsMock.mockResolvedValue([CHAIN_ID_MOCK]); + + 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 = { + ...mockRequest, + method, + params, + }; + await PPOMUtil.validateRequest(request, CHAIN_ID_MOCK); + + expect(validateWithSecurityAlertsAPIMock).toHaveBeenCalledTimes(1); + expect(validateWithSecurityAlertsAPIMock).toHaveBeenCalledWith( + CHAIN_ID_MOCK, + { + ...request, + params: firstTwoParams, + }, + ); + }, + ); }); }); diff --git a/app/lib/ppom/ppom-util.ts b/app/lib/ppom/ppom-util.ts index 3a7716eb87f..b76d4bf8f4b 100644 --- a/app/lib/ppom/ppom-util.ts +++ b/app/lib/ppom/ppom-util.ts @@ -34,6 +34,8 @@ export interface PPOMRequest { const TRANSACTION_METHOD = 'eth_sendTransaction'; const TRANSACTION_METHODS = [TRANSACTION_METHOD, 'eth_sendRawTransaction']; +export const METHOD_SIGN_TYPED_DATA_V3 = 'eth_signTypedData_v3'; +export const METHOD_SIGN_TYPED_DATA_V4 = 'eth_signTypedData_v4'; const CONFIRMATION_METHODS = Object.freeze([ 'eth_sendRawTransaction', @@ -155,7 +157,7 @@ async function validateWithController( ppomController: PPOMController, request: PPOMRequest, ): Promise { - try{ + try { const response = (await ppomController.usePPOM((ppom) => ppom.validateJsonRpc(request as unknown as Record), )) as SecurityAlertResponse; @@ -166,7 +168,10 @@ async function validateWithController( }; } catch (e) { Logger.log(`Error validating request with PPOM: ${e}`); - return {...SECURITY_ALERT_RESPONSE_FAILED, source: SecurityAlertSource.Local,}; + return { + ...SECURITY_ALERT_RESPONSE_FAILED, + source: SecurityAlertSource.Local, + }; } } @@ -212,9 +217,25 @@ function isTransactionRequest(request: PPOMRequest) { return TRANSACTION_METHODS.includes(request.method); } +function sanitizeRequest(request: PPOMRequest): PPOMRequest { + // 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 normalizeRequest(request: PPOMRequest): PPOMRequest { if (request.method !== TRANSACTION_METHOD) { - return request; + return sanitizeRequest(request); } request.origin = request.origin