Skip to content

Commit 6dcf6b3

Browse files
wantedsystemwantedsystem
authored
Merge pull request #70 from MetaMask/feat/stellar-snap-security-scan
feat: add security scanning to snap confirmations
2 parents 25f0b4a + fc1cd04 commit 6dcf6b3

34 files changed

Lines changed: 2096 additions & 27 deletions

‎packages/snap/.env.example‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ STATIC_API_BASE_URL=https://static.api.cx.metamask.io
4040
# Price API Base URL
4141
PRICE_API_BASE_URL=https://price.api.cx.metamask.io
4242

43+
# Security Alerts API Base URL
44+
SECURITY_ALERTS_API_BASE_URL=https://security-alerts.api.cx.metamask.io
45+
4346
# Cache TTL Milliseconds for base fee
4447
#BASE_FEE_TTL_MILLISECONDS=
4548

‎packages/snap/locales/en.json‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,24 +100,54 @@
100100
"confirmation.estimatedChanges.unsupportedContract": {
101101
"message": "Unsupported contract for simulation"
102102
},
103-
"confirmation.simulationTitleAPIError": {
104-
"message": "Because of an error, we couldn't check for security alerts."
103+
"confirmation.securityScanInProgressTitle": {
104+
"message": "Checking for security issues"
105105
},
106-
"confirmation.simulationMessageAPIError": {
106+
"confirmation.securityScanInProgressMessage": {
107+
"message": "This usually takes a few seconds."
108+
},
109+
"confirmation.securityScanAPIErrorTitle": {
110+
"message": "Security scan failed"
111+
},
112+
"confirmation.securityScanAPIErrorMessage": {
107113
"message": "Only continue if you trust every address involved."
108114
},
115+
"confirmation.securityScanErrorTitle": {
116+
"message": "Security scan failed"
117+
},
118+
"confirmation.securityScanErrorSubtitle": {
119+
"message": "{reason}"
120+
},
121+
"confirmation.securityScanIncompleteTitle": {
122+
"message": "Security scan incomplete"
123+
},
124+
"confirmation.securityScanIncompleteSubtitle": {
125+
"message": "{reason}. Only continue if you trust every address involved."
126+
},
109127
"confirmation.simulationErrorTitle": {
110128
"message": "This transaction was reverted during simulation."
111129
},
112130
"confirmation.simulationErrorSubtitle": {
113131
"message": "{reason}"
114132
},
133+
"confirmation.validationScanErrorTitle": {
134+
"message": "Security validation failed"
135+
},
136+
"confirmation.validationScanErrorSubtitle": {
137+
"message": "{reason}"
138+
},
115139
"confirmation.validationErrorTitle": {
116140
"message": "This is a deceptive request"
117141
},
118142
"confirmation.validationErrorSubtitle": {
119143
"message": "If you approve this request, a third party known for scams will take all your assets."
120144
},
145+
"confirmation.validationWarningTitle": {
146+
"message": "This request may be risky"
147+
},
148+
"confirmation.validationWarningSubtitle": {
149+
"message": "Security Alerts found potential risk. Only continue if you trust this site and every address involved."
150+
},
121151
"confirmation.validationErrorLearnMore": {
122152
"message": "Learn more"
123153
},

‎packages/snap/locales/es.json‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,24 +88,54 @@
8888
"confirmation.estimatedChanges.unsupportedContract": {
8989
"message": "Unsupported contract for simulation"
9090
},
91-
"confirmation.simulationTitleAPIError": {
92-
"message": "Because of an error, we couldn't check for security alerts."
91+
"confirmation.securityScanInProgressTitle": {
92+
"message": "Checking for security issues"
9393
},
94-
"confirmation.simulationMessageAPIError": {
94+
"confirmation.securityScanInProgressMessage": {
95+
"message": "This usually takes a few seconds."
96+
},
97+
"confirmation.securityScanAPIErrorTitle": {
98+
"message": "Security scan failed"
99+
},
100+
"confirmation.securityScanAPIErrorMessage": {
95101
"message": "Only continue if you trust every address involved."
96102
},
103+
"confirmation.securityScanErrorTitle": {
104+
"message": "Security scan failed"
105+
},
106+
"confirmation.securityScanErrorSubtitle": {
107+
"message": "{reason}"
108+
},
109+
"confirmation.securityScanIncompleteTitle": {
110+
"message": "Security scan incomplete"
111+
},
112+
"confirmation.securityScanIncompleteSubtitle": {
113+
"message": "{reason}. Only continue if you trust every address involved."
114+
},
97115
"confirmation.simulationErrorTitle": {
98116
"message": "This transaction was reverted during simulation."
99117
},
100118
"confirmation.simulationErrorSubtitle": {
101119
"message": "{reason}"
102120
},
121+
"confirmation.validationScanErrorTitle": {
122+
"message": "Security validation failed"
123+
},
124+
"confirmation.validationScanErrorSubtitle": {
125+
"message": "{reason}"
126+
},
103127
"confirmation.validationErrorTitle": {
104128
"message": "This is a deceptive request"
105129
},
106130
"confirmation.validationErrorSubtitle": {
107131
"message": "If you approve this request, a third party known for scams will take all your assets."
108132
},
133+
"confirmation.validationWarningTitle": {
134+
"message": "This request may be risky"
135+
},
136+
"confirmation.validationWarningSubtitle": {
137+
"message": "Security Alerts found potential risk. Only continue if you trust this site and every address involved."
138+
},
109139
"confirmation.validationErrorLearnMore": {
110140
"message": "Learn more"
111141
},

‎packages/snap/messages.json‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,24 +98,54 @@
9898
"confirmation.estimatedChanges.unsupportedContract": {
9999
"message": "Unsupported contract for simulation"
100100
},
101-
"confirmation.simulationTitleAPIError": {
102-
"message": "Because of an error, we couldn't check for security alerts."
101+
"confirmation.securityScanInProgressTitle": {
102+
"message": "Checking for security issues"
103103
},
104-
"confirmation.simulationMessageAPIError": {
104+
"confirmation.securityScanInProgressMessage": {
105+
"message": "This usually takes a few seconds."
106+
},
107+
"confirmation.securityScanAPIErrorTitle": {
108+
"message": "Security scan failed"
109+
},
110+
"confirmation.securityScanAPIErrorMessage": {
105111
"message": "Only continue if you trust every address involved."
106112
},
113+
"confirmation.securityScanErrorTitle": {
114+
"message": "Security scan failed"
115+
},
116+
"confirmation.securityScanErrorSubtitle": {
117+
"message": "{reason}"
118+
},
119+
"confirmation.securityScanIncompleteTitle": {
120+
"message": "Security scan incomplete"
121+
},
122+
"confirmation.securityScanIncompleteSubtitle": {
123+
"message": "{reason}. Only continue if you trust every address involved."
124+
},
107125
"confirmation.simulationErrorTitle": {
108126
"message": "This transaction was reverted during simulation."
109127
},
110128
"confirmation.simulationErrorSubtitle": {
111129
"message": "{reason}"
112130
},
131+
"confirmation.validationScanErrorTitle": {
132+
"message": "Security validation failed"
133+
},
134+
"confirmation.validationScanErrorSubtitle": {
135+
"message": "{reason}"
136+
},
113137
"confirmation.validationErrorTitle": {
114138
"message": "This is a deceptive request"
115139
},
116140
"confirmation.validationErrorSubtitle": {
117141
"message": "If you approve this request, a third party known for scams will take all your assets."
118142
},
143+
"confirmation.validationWarningTitle": {
144+
"message": "This request may be risky"
145+
},
146+
"confirmation.validationWarningSubtitle": {
147+
"message": "Security Alerts found potential risk. Only continue if you trust this site and every address involved."
148+
},
119149
"confirmation.validationErrorLearnMore": {
120150
"message": "Learn more"
121151
},

‎packages/snap/snap.config.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ const config: SnapConfig = {
2626
STATIC_API_BASE_URL: process.env.STATIC_API_BASE_URL ?? '',
2727
PRICE_API_BASE_URL: process.env.PRICE_API_BASE_URL ?? '',
2828
PRICE_API_CHUNK_SIZE: process.env.PRICE_API_CHUNK_SIZE ?? '',
29+
SECURITY_ALERTS_API_BASE_URL:
30+
process.env.SECURITY_ALERTS_API_BASE_URL ?? '',
2931
FIAT_EXCHANGE_RATES_TTL_MILLISECONDS:
3032
process.env.FIAT_EXCHANGE_RATES_TTL_MILLISECONDS ?? '',
3133
HISTORICAL_PRICES_TTL_MILLISECONDS:

‎packages/snap/snap.manifest.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"url": "https://github.com/MetaMask/snap-stellar-wallet.git"
88
},
99
"source": {
10-
"shasum": "MpounG90lvEVTjh6YZBuZ2NHs0dUNlU4XO6odOBAygs=",
10+
"shasum": "Pgeb7G9usrwUVpMdMU0LaszaxXGICctagd9YBNAexY0=",
1111
"location": {
1212
"npm": {
1313
"filePath": "dist/bundle.js",

‎packages/snap/src/config.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ import {
2020
KnownCaip2ChainId,
2121
} from './api';
2222

23+
const DEFAULT_SECURITY_ALERTS_API_BASE_URL =
24+
'https://security-alerts.api.cx.metamask.io';
25+
2326
/**
2427
* A struct to parse an integer from a string.
2528
*
@@ -100,6 +103,9 @@ const ConfigStruct = object({
100103
baseUrl: UrlStruct,
101104
chunkSize: parseIntegerStruct(1, 20),
102105
}),
106+
securityAlertsApi: object({
107+
baseUrl: UrlStruct,
108+
}),
103109
}),
104110
cache: object({
105111
ttlMilliseconds: object({
@@ -171,6 +177,13 @@ export const AppConfig = create(
171177
baseUrl: process.env.PRICE_API_BASE_URL,
172178
chunkSize: process.env.PRICE_API_CHUNK_SIZE,
173179
},
180+
securityAlertsApi: {
181+
baseUrl:
182+
process.env.SECURITY_ALERTS_API_BASE_URL === ''
183+
? DEFAULT_SECURITY_ALERTS_API_BASE_URL
184+
: (process.env.SECURITY_ALERTS_API_BASE_URL ??
185+
DEFAULT_SECURITY_ALERTS_API_BASE_URL),
186+
},
174187
},
175188
cache: {
176189
ttlMilliseconds: {

‎packages/snap/src/context.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type { ICronjobRequestHandler } from './handlers/cronjob/api';
1919
import { BackgroundEventMethod } from './handlers/cronjob/api';
2020
import {
2121
ConfirmationPriceRefresher,
22+
ConfirmationScanRefresher,
2223
RefreshConfirmationContextHandler,
2324
} from './handlers/cronjob/refreshConfirmationContext';
2425
import { SyncAccountsHandler } from './handlers/cronjob/syncAccounts';
@@ -49,6 +50,10 @@ import {
4950
TransactionRepository,
5051
TransactionService,
5152
} from './services/transaction';
53+
import {
54+
SecurityAlertsApiClient,
55+
TransactionScanService,
56+
} from './services/transaction-scan';
5257
import { WalletService } from './services/wallet';
5358
import { ConfirmationUXController } from './ui/confirmation/controller';
5459
import { logger, noOpLogger } from './utils';
@@ -111,6 +116,13 @@ const priceService = new PriceService({
111116
logger,
112117
});
113118

119+
const transactionScanService = new TransactionScanService({
120+
securityAlertsApiClient: new SecurityAlertsApiClient(
121+
AppConfig.api.securityAlertsApi,
122+
),
123+
logger,
124+
});
125+
114126
/** UX Controller */
115127
const confirmationUIController = new ConfirmationUXController({
116128
logger,
@@ -170,11 +182,16 @@ const confirmationPriceRefresher = new ConfirmationPriceRefresher({
170182
priceService,
171183
});
172184

185+
const confirmationScanRefresher = new ConfirmationScanRefresher({
186+
logger,
187+
transactionScanService,
188+
});
189+
173190
const refreshConfirmationContextHandler = new RefreshConfirmationContextHandler(
174191
{
175192
logger,
176193
confirmationUIController,
177-
refreshers: [confirmationPriceRefresher],
194+
refreshers: [confirmationPriceRefresher, confirmationScanRefresher],
178195
},
179196
);
180197

‎packages/snap/src/handlers/clientRequest/changeTrustOpt.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,14 @@ describe('ChangeTrustOptHandler', () => {
235235
account,
236236
assetMetadata,
237237
},
238+
renderOptions: {
239+
loadPrice: true,
240+
scanTxn: true,
241+
},
242+
securityScanRequest: {
243+
accountAddress: account.address,
244+
transaction: expect.any(String),
245+
},
238246
}),
239247
);
240248
const signedTransaction = signTransactionSpy.mock.calls[0]?.[0];
@@ -340,6 +348,14 @@ describe('ChangeTrustOptHandler', () => {
340348
expect(renderConfirmationDialog).toHaveBeenCalledWith(
341349
expect.objectContaining({
342350
interfaceKey: ConfirmationInterfaceKey.ChangeTrustlineOptOut,
351+
renderOptions: {
352+
loadPrice: true,
353+
scanTxn: true,
354+
},
355+
securityScanRequest: {
356+
accountAddress: account.address,
357+
transaction: expect.any(String),
358+
},
343359
}),
344360
);
345361
expect(sendTransaction).toHaveBeenCalled();

‎packages/snap/src/handlers/clientRequest/changeTrustOpt.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ export class ChangeTrustOptHandler extends BaseClientRequestHandler<
131131
assetMetadata,
132132
fee: transaction.totalFee.toString(),
133133
action,
134+
transaction,
134135
});
135136

136137
if (!confirmed) {
@@ -208,6 +209,7 @@ export class ChangeTrustOptHandler extends BaseClientRequestHandler<
208209
assetMetadata: StellarAssetMetadata;
209210
fee: string;
210211
action: ChangeTrustOptAction;
212+
transaction: Transaction;
211213
}): Promise<boolean> {
212214
return params.action === ChangeTrustOptAction.Delete
213215
? await this.#confirmSignChangeTrustOptOut(params)
@@ -219,6 +221,7 @@ export class ChangeTrustOptHandler extends BaseClientRequestHandler<
219221
account: StellarKeyringAccount;
220222
assetMetadata: StellarAssetMetadata;
221223
fee: string;
224+
transaction: Transaction;
222225
}): Promise<boolean> {
223226
return this.#confirmSignChangeTrust({
224227
...params,
@@ -231,6 +234,7 @@ export class ChangeTrustOptHandler extends BaseClientRequestHandler<
231234
account: StellarKeyringAccount;
232235
assetMetadata: StellarAssetMetadata;
233236
fee: string;
237+
transaction: Transaction;
234238
}): Promise<boolean> {
235239
return this.#confirmSignChangeTrust({
236240
...params,
@@ -243,6 +247,7 @@ export class ChangeTrustOptHandler extends BaseClientRequestHandler<
243247
account: StellarKeyringAccount;
244248
assetMetadata: StellarAssetMetadata;
245249
fee: string;
250+
transaction: Transaction;
246251
confirmationInterfaceKey:
247252
| ConfirmationInterfaceKey.ChangeTrustlineOptIn
248253
| ConfirmationInterfaceKey.ChangeTrustlineOptOut;
@@ -254,6 +259,7 @@ export class ChangeTrustOptHandler extends BaseClientRequestHandler<
254259
account,
255260
assetMetadata,
256261
fee,
262+
transaction,
257263
confirmationInterfaceKey,
258264
} = params;
259265
return (
@@ -267,6 +273,11 @@ export class ChangeTrustOptHandler extends BaseClientRequestHandler<
267273
interfaceKey: confirmationInterfaceKey,
268274
renderOptions: {
269275
loadPrice: true,
276+
scanTxn: true,
277+
},
278+
securityScanRequest: {
279+
accountAddress: account.address,
280+
transaction: transaction.getRaw().toXDR(),
270281
},
271282
})) === true
272283
);

0 commit comments

Comments
 (0)