Skip to content

Commit 76c5179

Browse files
shane-tshane-t
committed
chore: validate that custodians did not change parameters when signing
1 parent 895209c commit 76c5179

File tree

5 files changed

+154
-17
lines changed

5 files changed

+154
-17
lines changed

‎packages/snap/src/features/error-message/ErrorMessage.tsx

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { Box, Text } from '@metamask/snaps-sdk/jsx';
2+
import type { SnapComponent, SnapElement } from '@metamask/snaps-sdk/jsx';
3+
4+
export type ErrorMessageProps = {
5+
errorMessage?: string;
6+
};
7+
8+
export const ErrorMessage: SnapComponent<ErrorMessageProps> = ({
9+
errorMessage,
10+
}: ErrorMessageProps): SnapElement => {
11+
return (
12+
<Box>
13+
<Text>{errorMessage ?? 'An error occurred'}</Text>
14+
</Box>
15+
) as SnapElement;
16+
};

‎packages/snap/src/features/error-message/render.tsx

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { DialogType } from '@metamask/snaps-sdk';
2+
3+
import { ErrorMessage } from './ErrorMessage';
4+
import { createInterface, showDialog } from '../../lib/helpers/interface';
5+
/**
6+
* Renders the onboarding interface.
7+
*
8+
* @param errorMessage - The error message to display.
9+
* @returns The result of the dialog.
10+
*/
11+
export async function renderErrorMessage(errorMessage: string) {
12+
const id = await createInterface(
13+
<ErrorMessage errorMessage={errorMessage} />,
14+
{},
15+
);
16+
17+
await showDialog(id, DialogType.Alert);
18+
}

‎packages/snap/src/lib/helpers/interface.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type {
22
ComponentOrElement,
33
DialogResult,
4+
DialogType,
45
GetInterfaceStateResult,
56
Json,
67
ResolveInterfaceResult,
@@ -91,13 +92,18 @@ export async function resolveInterface(
9192
* Shows a dialog using the provided ID.
9293
*
9394
* @param id - The ID for the dialog.
95+
* @param type - The type of dialog to display.
9496
* @returns A promise that resolves to a string.
9597
*/
96-
export async function showDialog(id: string): Promise<DialogResult> {
98+
export async function showDialog(
99+
id: string,
100+
type?: DialogType,
101+
): Promise<DialogResult> {
97102
return snap.request({
98103
method: 'snap_dialog',
99104
params: {
100105
id,
106+
...(type ? { type } : {}),
101107
},
102108
});
103109
}

‎packages/snap/src/lib/helpers/transaction.ts

+87-10
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import { TransactionFactory } from '@ethereumjs/tx';
1+
import { TransactionFactory, TypedTransaction } from '@ethereumjs/tx';
22

33
import logger from '../../logger';
44
import { createCommon, formatTransactionData } from '../../util';
55
import { hexlify } from '../../util/hexlify';
66
import { TRANSACTION_TYPES } from '../constants';
7+
import type { EthSignTransactionRequest } from '../types/EthSignTransactionRequest';
78
import type { ITransactionDetails } from '../types/ITransactionDetails';
89
import type { IEIP1559TxParams, ILegacyTXParams } from '../types/ITXParams';
910

@@ -38,19 +39,11 @@ export class TransactionHelper {
3839
transaction: ITransactionDetails,
3940
chainId: string,
4041
): Promise<{ v: string; r: string; s: string }> {
41-
const common = createCommon(transaction, chainId);
4242

4343
if (transaction.signedRawTransaction) {
4444
logger.info('Transaction is signed', transaction.signedRawTransaction);
45-
const signedRawTransaction = Buffer.from(
46-
transaction.signedRawTransaction.substring(2),
47-
'hex',
48-
);
49-
50-
const tx = TransactionFactory.fromSerializedData(signedRawTransaction, {
51-
common,
52-
});
5345

46+
const tx = this.getTypedTransaction(transaction, chainId);
5447
return {
5548
v: hexlify(tx.v?.toString() ?? '0'),
5649
r: hexlify(tx.r?.toString() ?? '0'),
@@ -78,4 +71,88 @@ export class TransactionHelper {
7871
s: tx.s,
7972
};
8073
}
74+
75+
static getTypedTransaction(
76+
transactionDetails: ITransactionDetails,
77+
chainId: string,
78+
): TypedTransaction {
79+
if (!transactionDetails.signedRawTransaction) {
80+
throw new Error('Transaction is not signed');
81+
}
82+
83+
const common = createCommon(transactionDetails, chainId);
84+
85+
const signedRawTransaction = Buffer.from(
86+
transactionDetails.signedRawTransaction.substring(2),
87+
'hex',
88+
);
89+
90+
const tx = TransactionFactory.fromSerializedData(signedRawTransaction, {
91+
common,
92+
});
93+
94+
return tx;
95+
}
96+
97+
/**
98+
* We need to be certain that the custodian did not alter any of the fields
99+
* which was previously allowed in MMI but cannot work in the institutional snap
100+
* because the snap keyring does not allow us to return anything other than the signature
101+
*
102+
* @param request - The original request we got from the keyring
103+
* @param transactionDetails - The transaction details we got from the custodian
104+
*/
105+
106+
static validateTransaction(
107+
request: EthSignTransactionRequest,
108+
transactionDetails: ITransactionDetails,
109+
): {
110+
isValid: boolean;
111+
error?: string;
112+
} {
113+
const tx = this.getTypedTransaction(transactionDetails, request.chainId);
114+
// Validated nonce, gas price, maxFeePerGas, maxPriorityFeePerGas and gas limit
115+
if (Number(request.nonce) !== Number(tx.nonce)) {
116+
return {
117+
isValid: false,
118+
error: `Custodian altered the nonce from the request: ${request.nonce} to ${tx.nonce}`,
119+
};
120+
}
121+
if (
122+
'gasPrice' in request &&
123+
'gasPrice' in tx &&
124+
Number(request.gasPrice) !== Number(tx.gasPrice)
125+
) {
126+
return {
127+
isValid: false,
128+
error: `Custodian altered the gas price from the request: ${request.gasPrice} to ${tx.gasPrice}`,
129+
};
130+
}
131+
132+
if (
133+
'maxFeePerGas' in request &&
134+
'maxFeePerGas' in tx &&
135+
Number(request.maxFeePerGas) !== Number(tx.maxFeePerGas)
136+
) {
137+
return {
138+
isValid: false,
139+
error: `Custodian altered the maxFeePerGas from the request: ${request.maxFeePerGas} to ${tx.maxFeePerGas}`,
140+
};
141+
}
142+
143+
if (
144+
'maxPriorityFeePerGas' in request &&
145+
'maxPriorityFeePerGas' in tx &&
146+
Number(request.maxPriorityFeePerGas) !== Number(tx.maxPriorityFeePerGas)
147+
) {
148+
return {
149+
isValid: false,
150+
error: `Custodian altered the maxPriorityFeePerGas from the request: ${request.maxPriorityFeePerGas} to ${tx.maxPriorityFeePerGas}`,
151+
};
152+
}
153+
154+
return {
155+
isValid: true,
156+
};
157+
}
81158
}

‎packages/snap/src/requestsManager.ts

+26-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { emitSnapKeyringEvent, KeyringEvent } from '@metamask/keyring-api';
22
import type { Json } from '@metamask/snaps-sdk';
33

4+
import { renderErrorMessage } from './features/error-message/render';
45
import { TransactionHelper } from './lib/helpers/transaction';
56
import type { ICustodianApi } from './lib/types';
67
import type {
@@ -48,6 +49,15 @@ export class RequestManager {
4849
}
4950

5051
async getChainIdFromPendingRequest(id: string): Promise<string> {
52+
const transactionRequest = this.getRequestParams(id);
53+
if (!transactionRequest.chainId) {
54+
throw new Error(`Request ${id} has no chainId`);
55+
}
56+
57+
return transactionRequest.chainId;
58+
}
59+
60+
getRequestParams(id: string): EthSignTransactionRequest {
5161
if (!this.#state.requests[id]) {
5262
throw new Error(`Request ${id} not found`);
5363
}
@@ -59,12 +69,7 @@ export class RequestManager {
5969
throw new Error(`Request ${id} has invalid params`);
6070
}
6171

62-
const transactionRequest = requestParams[0] as EthSignTransactionRequest;
63-
if (!transactionRequest.chainId) {
64-
throw new Error(`Request ${id} has no chainId`);
65-
}
66-
67-
return transactionRequest.chainId;
72+
return requestParams[0] as EthSignTransactionRequest;
6873
}
6974

7075
async clearAllRequests(): Promise<void> {
@@ -145,6 +150,21 @@ export class RequestManager {
145150
chainId,
146151
);
147152

153+
const validationResult = TransactionHelper.validateTransaction(
154+
this.getRequestParams(requestId),
155+
transactionResponse,
156+
);
157+
158+
if (!validationResult.isValid) {
159+
// First show a dialog with the error message
160+
if (validationResult.error) {
161+
await renderErrorMessage(validationResult.error);
162+
}
163+
await this.emitRejectedEvent(requestId);
164+
await this.removePendingRequest(requestId);
165+
return;
166+
}
167+
148168
const updatedTransaction = {
149169
...request,
150170
fulfilled: true,

0 commit comments

Comments
 (0)