Skip to content

Commit

Permalink
fix: normalize transaction parameters before PPOM validation (#8774)
Browse files Browse the repository at this point in the history
Normalize transaction parameters before validating with the PPOM to
ensure the `data` can be parsed correctly, and all parameters are in a
consistent format.
  • Loading branch information
matthewwalsh0 authored Mar 4, 2024
1 parent 3983c18 commit 2c629fa
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 16 deletions.
46 changes: 46 additions & 0 deletions app/lib/ppom/ppom-util.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { normalizeTransactionParams } from '@metamask/transaction-controller';
import * as SignatureRequestActions from '../../actions/signatureRequest'; // eslint-disable-line import/no-namespace
import * as TransactionActions from '../../actions/transaction'; // eslint-disable-line import/no-namespace
import Engine from '../../core/Engine';
Expand Down Expand Up @@ -27,6 +28,11 @@ jest.mock('../../core/Engine', () => ({
},
}));

jest.mock('@metamask/transaction-controller', () => ({
...jest.requireActual('@metamask/transaction-controller'),
normalizeTransactionParams: jest.fn(),
}));

const mockRequest = {
id: 4247010338,
jsonrpc: '2.0',
Expand Down Expand Up @@ -59,9 +65,15 @@ const mockSignatureRequest = {
};

describe('validateResponse', () => {
const normalizeTransactionParamsMock = jest.mocked(
normalizeTransactionParams,
);

beforeEach(() => {
Engine.context.PreferencesController.state.securityAlertsEnabled = true;
Engine.context.NetworkController.state.providerConfig.chainId = '0x1';

normalizeTransactionParamsMock.mockImplementation((params) => params);
});

afterEach(() => {
Expand Down Expand Up @@ -137,4 +149,38 @@ describe('validateResponse', () => {
await PPOMUtil.validateRequest(mockSignatureRequest);
expect(spy).toBeCalledTimes(2);
});

it('normalizes transaction requests before validation', async () => {
const normalizedTransactionParamsMock = {
...mockRequest.params[0],
data: '0xabcd',
};

const validateMock = jest.fn();

const ppomMock = {
validateJsonRpc: validateMock,
};

Engine.context.PPOMController.usePPOM.mockImplementation((callback: any) =>
callback(ppomMock),
);

normalizeTransactionParamsMock.mockReturnValue(
normalizedTransactionParamsMock,
);

await PPOMUtil.validateRequest(mockRequest, '123');

expect(normalizeTransactionParamsMock).toBeCalledTimes(1);
expect(normalizeTransactionParamsMock).toBeCalledWith(
mockRequest.params[0],
);

expect(validateMock).toBeCalledTimes(1);
expect(validateMock).toBeCalledWith({
...mockRequest,
params: [normalizedTransactionParamsMock],
});
});
});
22 changes: 20 additions & 2 deletions app/lib/ppom/ppom-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ import { store } from '../../store';
import { isBlockaidFeatureEnabled } from '../../util/blockaid';
import Logger from '../../util/Logger';
import { updateSecurityAlertResponse } from '../../util/transaction-controller';
import { normalizeTransactionParams } from '@metamask/transaction-controller';

const TRANSACTION_METHOD = 'eth_sendTransaction';

const ConfirmationMethods = Object.freeze([
'eth_sendRawTransaction',
'eth_sendTransaction',
TRANSACTION_METHOD,
'eth_sign',
'eth_signTypedData',
'eth_signTypedData_v1',
Expand Down Expand Up @@ -71,8 +74,9 @@ const validateRequest = async (req: any, transactionId?: string) => {
setSignatureRequestSecurityAlertResponse(RequestInProgress),
);
}
const normalizedRequest = normalizeRequest(req);
securityAlertResponse = await ppomController.usePPOM((ppom: any) =>
ppom.validateJsonRpc(req),
ppom.validateJsonRpc(normalizedRequest),
);
securityAlertResponse = {
...securityAlertResponse,
Expand Down Expand Up @@ -110,4 +114,18 @@ const validateRequest = async (req: any, transactionId?: string) => {
return securityAlertResponse;
};

function normalizeRequest(request: any) {
if (request.method !== TRANSACTION_METHOD) {
return request;
}

const transactionParams = request.params?.[0] || {};
const normalizedParams = normalizeTransactionParams(transactionParams);

return {
...request,
params: [normalizedParams],
};
}

export default { validateRequest };
Loading

0 comments on commit 2c629fa

Please sign in to comment.