From f784171996dcd322893c75c11b2afe302f995c44 Mon Sep 17 00:00:00 2001 From: MetaMask Bot Date: Thu, 12 Dec 2024 18:09:54 +0000 Subject: [PATCH 1/7] Version v12.9.2 --- CHANGELOG.md | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9408b61ce731..9b292e7a0285 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [12.9.2] + ## [12.9.1] ### Changed - The 'All Networks' view of assets on the home screen will now only get data across the 9 'popular networks' ([#29071](https://github.com/MetaMask/metamask-extension/pull/29071)) @@ -5477,7 +5479,8 @@ Update styles and spacing on the critical error page ([#20350](https://github.c - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v12.9.1...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v12.9.2...HEAD +[12.9.2]: https://github.com/MetaMask/metamask-extension/compare/v12.9.1...v12.9.2 [12.9.1]: https://github.com/MetaMask/metamask-extension/compare/v12.9.0...v12.9.1 [12.9.0]: https://github.com/MetaMask/metamask-extension/compare/v12.8.1...v12.9.0 [12.8.1]: https://github.com/MetaMask/metamask-extension/compare/v12.8.0...v12.8.1 diff --git a/package.json b/package.json index 2717bfb589b3..446133b81a48 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "12.9.1", + "version": "12.9.2", "private": true, "repository": { "type": "git", From 21dc6ad4420cb576f24a4c8ae3b766c27f81358c Mon Sep 17 00:00:00 2001 From: OGPoyraz Date: Fri, 13 Dec 2024 11:32:47 +0100 Subject: [PATCH 2/7] chore: cherry-pick `29131` (#29185) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR cherry-picks https://github.com/MetaMask/metamask-extension/commit/acdf7c6579e2d1ac06e2ab4f2a0917d616df0403 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29185?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3783 ## **Manual testing steps** See original PR ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- .../transaction-details.test.tsx | 116 +++++++++++++++--- .../transaction-details.tsx | 15 ++- .../components/confirm/info/utils.test.ts | 107 +++++++++++++++- .../components/confirm/info/utils.ts | 83 +++++++++++++ 4 files changed, 300 insertions(+), 21 deletions(-) diff --git a/ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.test.tsx b/ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.test.tsx index 1263acf08397..f283ee6c4530 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.test.tsx @@ -1,7 +1,8 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; -import { SimulationErrorCode } from '@metamask/transaction-controller'; +import { Hex } from '@metamask/utils'; +import { toHex } from '@metamask/controller-utils'; import { getMockConfirmState, getMockConfirmStateForTransaction, @@ -44,21 +45,104 @@ describe('', () => { expect(container).toMatchSnapshot(); }); - it('renders component for transaction details with amount', () => { - const simulationDataMock = { - error: { code: SimulationErrorCode.Disabled }, - tokenBalanceChanges: [], - }; - const contractInteraction = genUnapprovedContractInteractionConfirmation({ - simulationData: simulationDataMock, - chainId: CHAIN_IDS.GOERLI, + describe('AmountRow', () => { + describe('should be in the document', () => { + it('when showAdvancedDetails is true', () => { + const contractInteraction = + genUnapprovedContractInteractionConfirmation({ + chainId: CHAIN_IDS.GOERLI, + }); + const state = getMockConfirmStateForTransaction(contractInteraction, { + metamask: { + preferences: { + showConfirmationAdvancedDetails: true, + }, + }, + }); + const mockStore = configureMockStore(middleware)(state); + const { getByTestId } = renderWithConfirmContextProvider( + , + mockStore, + ); + expect( + getByTestId('transaction-details-amount-row'), + ).toBeInTheDocument(); + }); + + it('when value and simulated native balance mismatch', () => { + // Transaction value is set to 0x3782dace9d900000 below mock + const simulationDataMock = { + tokenBalanceChanges: [], + nativeBalanceChange: { + difference: '0x1' as Hex, + isDecrease: false, + previousBalance: '0x2' as Hex, + newBalance: '0x1' as Hex, + }, + }; + const contractInteraction = + genUnapprovedContractInteractionConfirmation({ + simulationData: simulationDataMock, + chainId: CHAIN_IDS.GOERLI, + }); + const state = getMockConfirmStateForTransaction(contractInteraction, { + metamask: { + preferences: { + // Intentionally setting to false to test the condition + showConfirmationAdvancedDetails: false, + }, + }, + }); + const mockStore = configureMockStore(middleware)(state); + const { getByTestId } = renderWithConfirmContextProvider( + , + mockStore, + ); + expect( + getByTestId('transaction-details-amount-row'), + ).toBeInTheDocument(); + }); + }); + + it('should not be in the document when value and simulated native balance mismatch is within threshold', () => { + // Transaction value is set to 0x3782dace9d900000 below mock + const transactionValueInDecimal = 4000000000000000000; + const transactionValueInHex = toHex(transactionValueInDecimal); + const newBalanceInDecimal = 1; + const newBalanceInHex = toHex(newBalanceInDecimal); + const previousBalanceInDecimal = + transactionValueInDecimal + newBalanceInDecimal; + const previousBalanceInHex = toHex(previousBalanceInDecimal); + + const simulationDataMock = { + tokenBalanceChanges: [], + nativeBalanceChange: { + difference: transactionValueInHex, + isDecrease: true, + previousBalance: previousBalanceInHex, + newBalance: newBalanceInHex, + }, + }; + const contractInteraction = genUnapprovedContractInteractionConfirmation({ + simulationData: simulationDataMock, + chainId: CHAIN_IDS.GOERLI, + }); + const state = getMockConfirmStateForTransaction(contractInteraction, { + metamask: { + preferences: { + // Intentionally setting to false to test the condition + showConfirmationAdvancedDetails: false, + }, + }, + }); + const mockStore = configureMockStore(middleware)(state); + const { queryByTestId } = renderWithConfirmContextProvider( + , + mockStore, + ); + expect( + queryByTestId('transaction-details-amount-row'), + ).not.toBeInTheDocument(); }); - const state = getMockConfirmStateForTransaction(contractInteraction); - const mockStore = configureMockStore(middleware)(state); - const { getByTestId } = renderWithConfirmContextProvider( - , - mockStore, - ); - expect(getByTestId('transaction-details-amount-row')).toBeInTheDocument(); }); }); diff --git a/ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.tsx b/ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.tsx index 79cea5963c45..2cf6426f1975 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.tsx +++ b/ui/pages/confirmations/components/confirm/info/shared/transaction-details/transaction-details.tsx @@ -1,6 +1,6 @@ import { TransactionMeta } from '@metamask/transaction-controller'; import { isValidAddress } from 'ethereumjs-util'; -import React from 'react'; +import React, { useMemo } from 'react'; import { useSelector } from 'react-redux'; import { ConfirmInfoRow, @@ -20,6 +20,7 @@ import { ConfirmInfoRowCurrency } from '../../../../../../../components/app/conf import { PRIMARY } from '../../../../../../../helpers/constants/common'; import { useUserPreferencedCurrency } from '../../../../../../../hooks/useUserPreferencedCurrency'; import { HEX_ZERO } from '../constants'; +import { hasValueAndNativeBalanceMismatch as checkValueAndNativeBalanceMismatch } from '../../utils'; import { SigningInWithRow } from '../sign-in-with-row/sign-in-with-row'; export const OriginRow = () => { @@ -99,9 +100,8 @@ const AmountRow = () => { const { currency } = useUserPreferencedCurrency(PRIMARY); const value = currentConfirmation?.txParams?.value; - const simulationData = currentConfirmation?.simulationData; - if (!value || value === HEX_ZERO || !simulationData?.error) { + if (!value || value === HEX_ZERO) { return null; } @@ -150,6 +150,11 @@ export const TransactionDetails = () => { const showAdvancedDetails = useSelector( selectConfirmationAdvancedDetailsOpen, ); + const { currentConfirmation } = useConfirmContext(); + const hasValueAndNativeBalanceMismatch = useMemo( + () => checkValueAndNativeBalanceMismatch(currentConfirmation), + [currentConfirmation], + ); return ( <> @@ -159,7 +164,9 @@ export const TransactionDetails = () => { {showAdvancedDetails && } - + {(showAdvancedDetails || hasValueAndNativeBalanceMismatch) && ( + + )} ); diff --git a/ui/pages/confirmations/components/confirm/info/utils.test.ts b/ui/pages/confirmations/components/confirm/info/utils.test.ts index e78d06d87622..9c12b9127811 100644 --- a/ui/pages/confirmations/components/confirm/info/utils.test.ts +++ b/ui/pages/confirmations/components/confirm/info/utils.test.ts @@ -1,5 +1,10 @@ +import { TransactionMeta } from '@metamask/transaction-controller'; +import { toHex } from '@metamask/controller-utils'; import { DecodedTransactionDataSource } from '../../../../../../shared/types/transaction-decode'; -import { getIsRevokeSetApprovalForAll } from './utils'; +import { + getIsRevokeSetApprovalForAll, + hasValueAndNativeBalanceMismatch, +} from './utils'; describe('getIsRevokeSetApprovalForAll', () => { it('returns false if no data is passed as an argument', () => { @@ -36,3 +41,103 @@ describe('getIsRevokeSetApprovalForAll', () => { expect(actual).toEqual(true); }); }); + +describe('hasValueAndNativeBalanceMismatch', () => { + it('returns false when transaction value matches simulated balance change', () => { + const transactionValueInDecimal = 10000000000000000; + const transactionValueInHex = toHex(transactionValueInDecimal); + + const transaction = { + txParams: { + value: transactionValueInHex, + }, + simulationData: { + nativeBalanceChange: { + difference: transactionValueInHex, + isDecrease: true, + }, + }, + } as unknown as TransactionMeta; + + expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(false); + }); + + it('returns false when values differ within threshold', () => { + const transactionValueInDecimal = 10000000000000000; + const transactionValueInHex = toHex(transactionValueInDecimal); + + const differenceInDecimal = 10400000000000000; + const differenceInHex = toHex(differenceInDecimal); + + const transaction = { + txParams: { + value: transactionValueInHex, + }, + simulationData: { + nativeBalanceChange: { + difference: differenceInHex, + isDecrease: true, + }, + }, + } as unknown as TransactionMeta; + + expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(false); + }); + + it('returns true when values differ beyond threshold', () => { + const transactionValueInDecimal = 10000000000000000; + const transactionValueInHex = toHex(transactionValueInDecimal); + + const differenceInDecimal = 1000000000; + const muchSmallerDifferenceInHex = toHex(differenceInDecimal); + + const transaction = { + txParams: { + value: transactionValueInHex, + }, + simulationData: { + nativeBalanceChange: { + difference: muchSmallerDifferenceInHex, + isDecrease: true, + }, + }, + } as unknown as TransactionMeta; + + expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(true); + }); + + it('returns true when no simulation data is present', () => { + const transactionValueInDecimal = 10000000000000000; + const transactionValueInHex = toHex(transactionValueInDecimal); + + const transaction = { + txParams: { + value: transactionValueInHex, + }, + } as unknown as TransactionMeta; + + expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(true); + }); + + it('handles case when value is increased in simulation', () => { + const transactionValueInDecimal = 10000000000000000; + const transactionValueInHex = toHex(transactionValueInDecimal); + + const differenceInDecimal = 10000000000000000; + const differenceInHex = toHex(differenceInDecimal); + + const transaction = { + txParams: { + value: transactionValueInHex, + }, + simulationData: { + nativeBalanceChange: { + difference: differenceInHex, + isDecrease: false, + }, + }, + } as unknown as TransactionMeta; + + expect(hasValueAndNativeBalanceMismatch(transaction)).toBe(true); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/utils.ts b/ui/pages/confirmations/components/confirm/info/utils.ts index 0ad8479ac7b9..1af918aea74e 100644 --- a/ui/pages/confirmations/components/confirm/info/utils.ts +++ b/ui/pages/confirmations/components/confirm/info/utils.ts @@ -1,9 +1,15 @@ +import { TransactionMeta } from '@metamask/transaction-controller'; +import type { Hex } from '@metamask/utils'; +import { remove0x } from '@metamask/utils'; +import { BN } from 'bn.js'; import { DecodedTransactionDataResponse } from '../../../../../../shared/types/transaction-decode'; import { BackgroundColor, TextColor, } from '../../../../../helpers/constants/design-system'; +const VALUE_COMPARISON_PERCENT_THRESHOLD = 5; + export function getIsRevokeSetApprovalForAll( value: DecodedTransactionDataResponse | undefined, ): boolean { @@ -27,3 +33,80 @@ export const getAmountColors = (credit?: boolean, debit?: boolean) => { } return { color, backgroundColor }; }; + +/** + * Calculate the absolute percentage change between two values. + * + * @param originalValue - The first value. + * @param newValue - The second value. + * @returns The percentage change from the first value to the second value. + * If the original value is zero and the new value is not, returns 100. + */ +export function getPercentageChange( + originalValue: InstanceType, + newValue: InstanceType, +): number { + const precisionFactor = new BN(10).pow(new BN(18)); + const originalValuePrecision = originalValue.mul(precisionFactor); + const newValuePrecision = newValue.mul(precisionFactor); + + const difference = newValuePrecision.sub(originalValuePrecision); + + if (difference.isZero()) { + return 0; + } + + if (originalValuePrecision.isZero() && !newValuePrecision.isZero()) { + return 100; + } + + return difference.muln(100).div(originalValuePrecision).abs().toNumber(); +} + +/** + * Determine if the percentage change between two values is within a threshold. + * + * @param originalValue - The original value. + * @param newValue - The new value. + * @param newNegative - Whether the new value is negative. + * @returns Whether the percentage change between the two values is within a threshold. + */ +function percentageChangeWithinThreshold( + originalValue: Hex, + newValue: Hex, + newNegative?: boolean, +): boolean { + const originalValueBN = new BN(remove0x(originalValue), 'hex'); + let newValueBN = new BN(remove0x(newValue), 'hex'); + + if (newNegative) { + newValueBN = newValueBN.neg(); + } + + return ( + getPercentageChange(originalValueBN, newValueBN) <= + VALUE_COMPARISON_PERCENT_THRESHOLD + ); +} + +/** + * Determine if a transaction has a value and simulation native balance mismatch. + * + * @param transactionMeta - The transaction metadata. + * @returns Whether the transaction has a value and simulation native balance mismatch. + */ +export function hasValueAndNativeBalanceMismatch( + transactionMeta: TransactionMeta, +): boolean { + const value = transactionMeta?.txParams?.value ?? '0x0'; + const nativeBalanceChange = + transactionMeta?.simulationData?.nativeBalanceChange; + const simulatedNativeBalanceDifference = + nativeBalanceChange?.difference ?? '0x0'; + + return !percentageChangeWithinThreshold( + value as Hex, + simulatedNativeBalanceDifference, + nativeBalanceChange?.isDecrease === false, + ); +} From 37e51c10c78424ab7d1bec80078099278ead40f2 Mon Sep 17 00:00:00 2001 From: AugmentedMode <31675118+AugmentedMode@users.noreply.github.com> Date: Fri, 13 Dec 2024 05:40:50 -0500 Subject: [PATCH 3/7] fix: [cherry-pick] add websocket support for c2 detection (#28782) (#29173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cherry-picks #28782 ## **Description** This pull request adds WebSocket support to the MetaMask extension's phishing detection functionality. Scammers have started using WebSocket connections for command-and-control (C2) operations to bypass traditional HTTP-based phishing detection. This PR allows the extension to intercept and block WebSocket handshake requests (`ws://` and `wss://`) in addition to HTTP/HTTPS requests. The key changes include: 1. Adding WebSocket schemes (`ws://*/*` and `wss://*/*`) to the `urls` filter in `background.js`. 2. Updating the `manifest.json` to include WebSocket permissions in the `host_permissions` field. This ensures that malicious WebSocket connections can be detected and blocked. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28782?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3788 ## **Manual testing steps** 1. Navigate to `example.com` 2. Initiate a WebSocket connection to a known safe domain (e.g., `wss://example.com`) and verify it works as expected by going to the `console` via right clicking and hitting inspect. Then type into the console `new WebSocket("https://example.com/")` 3. Attempt a WebSocket connection to a domain flagged as phishing, and verify the connection is blocked and appropriate warnings are displayed by going to the `console` via right clicking and hitting inspect. Then type into the console `new WebSocket("https://walietconnectapi.com/")` ## **Screenshots/Recordings** ### **Before** No support for detecting WebSocket phishing connections. --- ### **After** WebSocket phishing connections are detected and blocked during the handshake phase. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. Co-authored-by: Mark Stacey --- app/manifest/v2/_base.json | 2 + app/manifest/v3/_base.json | 4 +- app/scripts/background.js | 17 ++-- privacy-snapshot.json | 3 +- test/e2e/helpers.js | 39 ++++++++++ test/e2e/tests/phishing-controller/mocks.js | 19 +++-- .../phishing-detection.spec.js | 77 ++++++++++++++++++- 7 files changed, 138 insertions(+), 23 deletions(-) diff --git a/app/manifest/v2/_base.json b/app/manifest/v2/_base.json index f29b7458a9e5..2f41a7e987fa 100644 --- a/app/manifest/v2/_base.json +++ b/app/manifest/v2/_base.json @@ -66,6 +66,8 @@ "clipboardWrite", "http://*/*", "https://*/*", + "ws://*/*", + "wss://*/*", "activeTab", "webRequest", "webRequestBlocking", diff --git a/app/manifest/v3/_base.json b/app/manifest/v3/_base.json index 4d6ee38437d3..89758033f33a 100644 --- a/app/manifest/v3/_base.json +++ b/app/manifest/v3/_base.json @@ -50,7 +50,9 @@ "http://localhost:8545/", "file://*/*", "http://*/*", - "https://*/*" + "https://*/*", + "ws://*/*", + "wss://*/*" ], "icons": { "16": "images/icon-16.png", diff --git a/app/scripts/background.js b/app/scripts/background.js index 2b2f5c4693df..938cac879f25 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -315,22 +315,21 @@ function maybeDetectPhishing(theController) { // blocking is better than tab redirection, as blocking will prevent // the browser from loading the page at all if (isManifestV2) { - if (details.type === 'sub_frame') { - // redirect the entire tab to the - // phishing warning page instead. - redirectTab(details.tabId, redirectHref); - // don't let the sub_frame load at all - return { cancel: true }; + // We can redirect `main_frame` requests directly to the warning page. + // For non-`main_frame` requests (e.g. `sub_frame` or WebSocket), we cancel them + // and redirect the whole tab asynchronously so that the user sees the warning. + if (details.type === 'main_frame') { + return { redirectUrl: redirectHref }; } - // redirect the whole tab - return { redirectUrl: redirectHref }; + redirectTab(details.tabId, redirectHref); + return { cancel: true }; } // redirect the whole tab (even if it's a sub_frame request) redirectTab(details.tabId, redirectHref); return {}; }, { - urls: ['http://*/*', 'https://*/*'], + urls: ['http://*/*', 'https://*/*', 'ws://*/*', 'wss://*/*'], }, isManifestV2 ? ['blocking'] : [], ); diff --git a/privacy-snapshot.json b/privacy-snapshot.json index 6ee430ca943c..8ae10de304df 100644 --- a/privacy-snapshot.json +++ b/privacy-snapshot.json @@ -71,5 +71,6 @@ "unresponsive-rpc.test", "unresponsive-rpc.url", "user-storage.api.cx.metamask.io", - "www.4byte.directory" + "www.4byte.directory", + "verify.walletconnect.com" ] diff --git a/test/e2e/helpers.js b/test/e2e/helpers.js index b06c29b17acf..6900ac243629 100644 --- a/test/e2e/helpers.js +++ b/test/e2e/helpers.js @@ -4,6 +4,7 @@ const BigNumber = require('bignumber.js'); const mockttp = require('mockttp'); const detectPort = require('detect-port'); const { difference } = require('lodash'); +const WebSocket = require('ws'); const createStaticServer = require('../../development/create-static-server'); const { setupMocking } = require('./mock-e2e'); const { Ganache } = require('./seeder/ganache'); @@ -640,6 +641,43 @@ async function unlockWallet( } } +/** + * Simulates a WebSocket connection by executing a script in the browser context. + * + * @param {WebDriver} driver - The WebDriver instance. + * @param {string} hostname - The hostname to connect to. + */ +async function createWebSocketConnection(driver, hostname) { + try { + await driver.executeScript(async (wsHostname) => { + const url = `ws://${wsHostname}:8000`; + const socket = new WebSocket(url); + socket.onopen = () => { + console.log('WebSocket connection opened'); + socket.send('Hello, server!'); + }; + socket.onerror = (error) => { + console.error( + 'WebSocket error:', + error.message || 'Connection blocked', + ); + }; + socket.onmessage = (event) => { + console.log('Message received from server:', event.data); + }; + socket.onclose = () => { + console.log('WebSocket connection closed'); + }; + }, hostname); + } catch (error) { + console.error( + `Failed to execute WebSocket connection script for ws://${hostname}:8000`, + error, + ); + throw error; + } +} + const logInWithBalanceValidation = async (driver, ganacheServer) => { await unlockWallet(driver); // Wait for balance to load @@ -975,4 +1013,5 @@ module.exports = { tempToggleSettingRedesignedTransactionConfirmations, openMenuSafe, sentryRegEx, + createWebSocketConnection, }; diff --git a/test/e2e/tests/phishing-controller/mocks.js b/test/e2e/tests/phishing-controller/mocks.js index fe11118c6fd2..3165847740bf 100644 --- a/test/e2e/tests/phishing-controller/mocks.js +++ b/test/e2e/tests/phishing-controller/mocks.js @@ -10,7 +10,9 @@ const { const lastUpdated = 1; const defaultHotlist = { data: [] }; const defaultC2DomainBlocklist = { - recentlyAdded: [], + recentlyAdded: [ + '33c8e026e76cea2df82322428554c932961cd80080fa379454350d7f13371f36', // hash for malicious.localhost + ], recentlyRemoved: [], lastFetchedAt: '2024-08-27T15:30:45Z', }; @@ -95,15 +97,12 @@ async function setupPhishingDetectionMocks( }; }); - await mockServer - .forGet(C2_DOMAIN_BLOCKLIST_URL) - .withQuery({ timestamp: '2024-08-27T15:30:45Z' }) - .thenCallback(() => { - return { - statusCode: 200, - json: defaultC2DomainBlocklist, - }; - }); + await mockServer.forGet(C2_DOMAIN_BLOCKLIST_URL).thenCallback(() => { + return { + statusCode: 200, + json: defaultC2DomainBlocklist, + }; + }); await mockServer .forGet('https://github.com/MetaMask/eth-phishing-detect/issues/new') diff --git a/test/e2e/tests/phishing-controller/phishing-detection.spec.js b/test/e2e/tests/phishing-controller/phishing-detection.spec.js index ad199cea1e70..1fabd8f901bc 100644 --- a/test/e2e/tests/phishing-controller/phishing-detection.spec.js +++ b/test/e2e/tests/phishing-controller/phishing-detection.spec.js @@ -9,6 +9,7 @@ const { openDapp, unlockWallet, WINDOW_TITLES, + createWebSocketConnection, } = require('../../helpers'); const FixtureBuilder = require('../../fixture-builder'); const { @@ -307,10 +308,82 @@ describe('Phishing Detection', function () { text: 'Back to safety', }); + await driver.waitForUrl({ + url: `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`, + }); + }, + ); + }); + + it('should block a website that makes a websocket connection to a malicious command and control server', async function () { + const testPageURL = 'http://localhost:8080'; + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions: defaultGanacheOptions, + title: this.test.fullTitle(), + testSpecificMock: async (mockServer) => { + await mockServer.forAnyWebSocket().thenEcho(); + await setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + }); + }, + dapp: true, + }, + async ({ driver }) => { + await unlockWallet(driver); + + await driver.openNewPage(testPageURL); + + await createWebSocketConnection(driver, 'malicious.localhost'); + + await driver.switchToWindowWithTitle( + 'MetaMask Phishing Detection', + 10000, + ); + + await driver.waitForSelector({ + testId: 'unsafe-continue-loaded', + }); + + await driver.clickElement({ + text: 'Back to safety', + }); + + await driver.waitForUrl({ + url: `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`, + }); + }, + ); + }); + + it('should not block a website that makes a safe WebSocket connection', async function () { + const testPageURL = 'http://localhost:8080/'; + await withFixtures( + { + fixtures: new FixtureBuilder().build(), + ganacheOptions: defaultGanacheOptions, + title: this.test.fullTitle(), + testSpecificMock: async (mockServer) => { + await mockServer.forAnyWebSocket().thenEcho(); + await setupPhishingDetectionMocks(mockServer, { + blockProvider: BlockProvider.MetaMask, + }); + }, + dapp: true, + }, + async ({ driver }) => { + await unlockWallet(driver); + + await driver.openNewPage(testPageURL); + + await createWebSocketConnection(driver, 'safe.localhost'); + + await driver.wait(until.titleIs(WINDOW_TITLES.TestDApp), 10000); + const currentUrl = await driver.getCurrentUrl(); - const expectedPortfolioUrl = `https://portfolio.metamask.io/?metamaskEntry=phishing_page_portfolio_button`; - assert.equal(currentUrl, expectedPortfolioUrl); + assert.equal(currentUrl, testPageURL); }, ); }); From 9f0571a286b691ace297109d0162232fc7d70571 Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Fri, 13 Dec 2024 02:41:34 -0800 Subject: [PATCH 4/7] chore(cherry-pick): Use correct selector to pull name from non-popular networks (#29164) cherry picks https://github.com/MetaMask/metamask-extension/pull/29121 to 12.9.2 Co-authored-by: Nick Gambino <35090461+gambinish@users.noreply.github.com> --- .../assets/asset-list/network-filter/network-filter.tsx | 2 +- .../multichain/token-list-item/token-list-item.tsx | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/ui/components/app/assets/asset-list/network-filter/network-filter.tsx b/ui/components/app/assets/asset-list/network-filter/network-filter.tsx index d08d01f933ee..a63aef334d11 100644 --- a/ui/components/app/assets/asset-list/network-filter/network-filter.tsx +++ b/ui/components/app/assets/asset-list/network-filter/network-filter.tsx @@ -203,7 +203,7 @@ const NetworkFilter = ({ handleClose }: SortControlProps) => { diff --git a/ui/components/multichain/token-list-item/token-list-item.tsx b/ui/components/multichain/token-list-item/token-list-item.tsx index 5ee4c19c8c52..34ef73e5b87b 100644 --- a/ui/components/multichain/token-list-item/token-list-item.tsx +++ b/ui/components/multichain/token-list-item/token-list-item.tsx @@ -45,7 +45,6 @@ import { getParticipateInMetaMetrics, getDataCollectionForMarketing, getMarketData, - getNetworkConfigurationIdByChainId, getCurrencyRates, } from '../../../selectors'; import { getMultichainIsEvm } from '../../../selectors/multichain'; @@ -69,6 +68,7 @@ import { SafeChain, useSafeChains, } from '../../../pages/settings/networks-tab/networks-form/use-safe-chains'; +import { getNetworkConfigurationsByChainId } from '../../../../shared/modules/selectors/networks'; import { PercentageChange } from './price/percentage-change/percentage-change'; type TokenListItemProps = { @@ -216,9 +216,7 @@ export const TokenListItem = ({ ); // Used for badge icon - const allNetworks: Record = useSelector( - getNetworkConfigurationIdByChainId, - ); + const allNetworks = useSelector(getNetworkConfigurationsByChainId); const testNetworkBackgroundColor = useSelector(getTestNetworkBackgroundColor); return ( @@ -270,7 +268,7 @@ export const TokenListItem = ({ badge={ Date: Fri, 13 Dec 2024 17:17:12 +0000 Subject: [PATCH 5/7] fix (cherry-pick): increase signing or submitting alert severity to danger (#29140) (#29192) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Cherry-pick of #29140 for release `12.9.2`. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29192?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/29138 ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --- app/_locales/de/messages.json | 3 - app/_locales/el/messages.json | 3 - app/_locales/en/messages.json | 3 - app/_locales/en_GB/messages.json | 3 - app/_locales/es/messages.json | 3 - app/_locales/fr/messages.json | 3 - app/_locales/hi/messages.json | 3 - app/_locales/id/messages.json | 3 - app/_locales/ja/messages.json | 3 - app/_locales/ko/messages.json | 3 - app/_locales/pt/messages.json | 3 - app/_locales/ru/messages.json | 3 - app/_locales/tl/messages.json | 3 - app/_locales/tr/messages.json | 3 - app/_locales/vi/messages.json | 3 - app/_locales/zh_CN/messages.json | 3 - .../transactions/alerts.test.tsx | 69 +++++++++++++++++++ .../components/confirm/footer/footer.test.tsx | 18 +++++ .../components/confirm/footer/footer.tsx | 4 +- .../useSigningOrSubmittingAlerts.test.ts | 5 +- .../useSigningOrSubmittingAlerts.ts | 4 +- 21 files changed, 93 insertions(+), 55 deletions(-) diff --git a/app/_locales/de/messages.json b/app/_locales/de/messages.json index 0c8ba19030f2..68434a86aefa 100644 --- a/app/_locales/de/messages.json +++ b/app/_locales/de/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Diese Seite fordert Sie auf, sich mit dem falschen Konto anzumelden." }, - "alertMessageSigningOrSubmitting": { - "message": "Diese Transaktion wird erst durchgeführt, wenn Ihre vorherige Transaktion abgeschlossen ist." - }, "alertModalAcknowledge": { "message": "Ich habe das Risiko erkannt und möchte trotzdem fortfahren" }, diff --git a/app/_locales/el/messages.json b/app/_locales/el/messages.json index 31cd6809080b..d5a3ddb277cd 100644 --- a/app/_locales/el/messages.json +++ b/app/_locales/el/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Αυτός ο ιστότοπος σας ζητάει να συνδεθείτε χρησιμοποιώντας λάθος λογαριασμό." }, - "alertMessageSigningOrSubmitting": { - "message": "Αυτή η συναλλαγή θα πραγματοποιηθεί μόνο όταν ολοκληρωθεί η προηγούμενη συναλλαγή σας." - }, "alertModalAcknowledge": { "message": "Αναγνωρίζω τον κίνδυνο και εξακολουθώ να θέλω να συνεχίσω" }, diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 49189816d2b3..12d84dea2c59 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -449,9 +449,6 @@ "alertMessageSignInWrongAccount": { "message": "This site is asking you to sign in using the wrong account." }, - "alertMessageSigningOrSubmitting": { - "message": "This transaction will only go through once your previous transaction is complete." - }, "alertModalAcknowledge": { "message": "I have acknowledged the risk and still want to proceed" }, diff --git a/app/_locales/en_GB/messages.json b/app/_locales/en_GB/messages.json index 92ad7c646a36..89ec6cc922e8 100644 --- a/app/_locales/en_GB/messages.json +++ b/app/_locales/en_GB/messages.json @@ -430,9 +430,6 @@ "alertMessageSignInWrongAccount": { "message": "This site is asking you to sign in using the wrong account." }, - "alertMessageSigningOrSubmitting": { - "message": "This transaction will only go through once your previous transaction is complete." - }, "alertModalAcknowledge": { "message": "I have acknowledged the risk and still want to proceed" }, diff --git a/app/_locales/es/messages.json b/app/_locales/es/messages.json index f2afe012534a..f114143ed7f0 100644 --- a/app/_locales/es/messages.json +++ b/app/_locales/es/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Este sitio le pide que inicie sesión con la cuenta incorrecta." }, - "alertMessageSigningOrSubmitting": { - "message": "Esta transacción solo se realizará una vez que se complete la transacción anterior." - }, "alertModalAcknowledge": { "message": "Soy consciente del riesgo y aun así deseo continuar" }, diff --git a/app/_locales/fr/messages.json b/app/_locales/fr/messages.json index 99c3a6da2333..77124ac6e041 100644 --- a/app/_locales/fr/messages.json +++ b/app/_locales/fr/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Ce site vous demande de vous connecter en utilisant le mauvais compte." }, - "alertMessageSigningOrSubmitting": { - "message": "La transaction précédente doit être finalisée avant que celle-ci ne soit traitée." - }, "alertModalAcknowledge": { "message": "Je suis conscient du risque et je souhaite quand même continuer" }, diff --git a/app/_locales/hi/messages.json b/app/_locales/hi/messages.json index 92d8d0bc7fa8..542da3a3f8de 100644 --- a/app/_locales/hi/messages.json +++ b/app/_locales/hi/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "यह साइट आपसे गलत अकाउंट का उपयोग करके साइन इन करने के लिए कह रही है।" }, - "alertMessageSigningOrSubmitting": { - "message": "यह ट्रांसेक्शन तभी पूरा होगा जब आपका पिछला ट्रांसेक्शन पूरा हो जाएगा।" - }, "alertModalAcknowledge": { "message": "मैंने जोखिम को स्वीकार कर लिया है और इसके बावजूद आगे बढ़ना चाहता/चाहती हूं" }, diff --git a/app/_locales/id/messages.json b/app/_locales/id/messages.json index a474fc1afa1a..63455b4d15bc 100644 --- a/app/_locales/id/messages.json +++ b/app/_locales/id/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Situs ini meminta Anda masuk menggunakan akun yang salah." }, - "alertMessageSigningOrSubmitting": { - "message": "Transaksi ini hanya akan dilanjutkan setelah transaksi Anda sebelumnya selesai." - }, "alertModalAcknowledge": { "message": "Saya telah mengetahui risikonya dan tetap ingin melanjutkan" }, diff --git a/app/_locales/ja/messages.json b/app/_locales/ja/messages.json index 80982f5b4144..4c218e06f3e7 100644 --- a/app/_locales/ja/messages.json +++ b/app/_locales/ja/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "このサイトは正しくないアカウントでのサインインを求めています。" }, - "alertMessageSigningOrSubmitting": { - "message": "このトランザクションは、前のトランザクションが完了しないと実行されません。" - }, "alertModalAcknowledge": { "message": "リスクを承知したうえで続行します" }, diff --git a/app/_locales/ko/messages.json b/app/_locales/ko/messages.json index 5fc7deb6a9ef..eb90361ab1ff 100644 --- a/app/_locales/ko/messages.json +++ b/app/_locales/ko/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "이 사이트에서 잘못된 계정으로 로그인하라고 요청합니다." }, - "alertMessageSigningOrSubmitting": { - "message": "이 트랜잭션은 이전 트랜잭션이 완료된 경우에만 진행됩니다." - }, "alertModalAcknowledge": { "message": "위험성을 인지했으며, 계속 진행합니다" }, diff --git a/app/_locales/pt/messages.json b/app/_locales/pt/messages.json index 18e1c97fb129..49fb6f6540c8 100644 --- a/app/_locales/pt/messages.json +++ b/app/_locales/pt/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Este site está pedindo que você entre usando a conta incorreta." }, - "alertMessageSigningOrSubmitting": { - "message": "Essa transação só será processada quando sua transação anterior for concluída." - }, "alertModalAcknowledge": { "message": "Eu reconheço o risco e ainda quero prosseguir" }, diff --git a/app/_locales/ru/messages.json b/app/_locales/ru/messages.json index 43e617a0aecd..ac6b7e78f60d 100644 --- a/app/_locales/ru/messages.json +++ b/app/_locales/ru/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Этот сайт просит вас войти в систему, используя неправильный счет." }, - "alertMessageSigningOrSubmitting": { - "message": "Эта транзакция будет выполнена только после завершения вашей предыдущей транзакции." - }, "alertModalAcknowledge": { "message": "Я осознал(-а) риск и все еще хочу продолжить" }, diff --git a/app/_locales/tl/messages.json b/app/_locales/tl/messages.json index 5bc72667356e..a0f7421f5f00 100644 --- a/app/_locales/tl/messages.json +++ b/app/_locales/tl/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Hinihingi sa iyo ng site na ito na mag-sign in gamit ang maling account." }, - "alertMessageSigningOrSubmitting": { - "message": "Magpapatuloy lamang ang transaksyong ito kapag nakumpleto mo na ang naunang transaksyon." - }, "alertModalAcknowledge": { "message": "Kinikilala ko ang panganib at nais ko pa rin magpatuloy" }, diff --git a/app/_locales/tr/messages.json b/app/_locales/tr/messages.json index a71a64576142..ea1382127920 100644 --- a/app/_locales/tr/messages.json +++ b/app/_locales/tr/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Bu site sizden yanlış hesabı kullanarak giriş yapmanızı istiyor." }, - "alertMessageSigningOrSubmitting": { - "message": "Bu işlem sadece önceki işleminiz tamamlandıktan sonra gerçekleşecek." - }, "alertModalAcknowledge": { "message": "Riski anlıyor ve yine de ilerlemek istiyorum" }, diff --git a/app/_locales/vi/messages.json b/app/_locales/vi/messages.json index 5fb6ee43bfb9..c7dadc95b378 100644 --- a/app/_locales/vi/messages.json +++ b/app/_locales/vi/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "Trang web này yêu cầu bạn đăng nhập bằng tài khoản không đúng." }, - "alertMessageSigningOrSubmitting": { - "message": "Giao dịch này sẽ chỉ được thực hiện sau khi giao dịch trước đó của bạn hoàn tất." - }, "alertModalAcknowledge": { "message": "Tôi đã nhận thức được rủi ro và vẫn muốn tiếp tục" }, diff --git a/app/_locales/zh_CN/messages.json b/app/_locales/zh_CN/messages.json index ca1d0e3f7b48..e64cdec5070e 100644 --- a/app/_locales/zh_CN/messages.json +++ b/app/_locales/zh_CN/messages.json @@ -443,9 +443,6 @@ "alertMessageSignInWrongAccount": { "message": "此网站要求您使用错误的账户登录。" }, - "alertMessageSigningOrSubmitting": { - "message": "您的上一笔交易完成后，此交易才能继续进行。" - }, "alertModalAcknowledge": { "message": "我已知晓风险并仍想继续" }, diff --git a/test/integration/confirmations/transactions/alerts.test.tsx b/test/integration/confirmations/transactions/alerts.test.tsx index 1bbf9d1fd2c5..fe78f3152bfe 100644 --- a/test/integration/confirmations/transactions/alerts.test.tsx +++ b/test/integration/confirmations/transactions/alerts.test.tsx @@ -411,4 +411,73 @@ describe('Contract Interaction Confirmation Alerts', () => { await screen.findByTestId('alert-modal-action-showGasFeeModal'), ).toHaveTextContent('Update gas options'); }); + + it('displays the alert for signing and submitting alerts', async () => { + const account = + mockMetaMaskState.internalAccounts.accounts[ + mockMetaMaskState.internalAccounts + .selectedAccount as keyof typeof mockMetaMaskState.internalAccounts.accounts + ]; + + const mockedMetaMaskState = + getMetaMaskStateWithUnapprovedApproveTransaction(account.address); + const unapprovedTransaction = mockedMetaMaskState.transactions[0]; + const signedTransaction = getUnapprovedApproveTransaction( + account.address, + randomUUID(), + pendingTransactionTime - 1000, + ); + signedTransaction.status = 'signed'; + + await act(async () => { + await integrationTestRender({ + preloadedState: { + ...mockedMetaMaskState, + gasEstimateType: 'none', + pendingApprovalCount: 2, + pendingApprovals: { + [pendingTransactionId]: { + id: pendingTransactionId, + origin: 'origin', + time: pendingTransactionTime, + type: ApprovalType.Transaction, + requestData: { + txId: pendingTransactionId, + }, + requestState: null, + expectsResult: false, + }, + [signedTransaction.id]: { + id: signedTransaction.id, + origin: 'origin', + time: pendingTransactionTime - 1000, + type: ApprovalType.Transaction, + requestData: { + txId: signedTransaction.id, + }, + requestState: null, + expectsResult: false, + }, + }, + transactions: [unapprovedTransaction, signedTransaction], + }, + backgroundConnection: backgroundConnectionMocked, + }); + }); + + const alerts = await screen.findAllByTestId('confirm-banner-alert'); + + expect( + alerts.some((alert) => + alert.textContent?.includes( + 'A previous transaction is still being signed or submitted', + ), + ), + ).toBe(true); + + expect( + await screen.findByTestId('confirm-footer-button'), + ).toBeInTheDocument(); + expect(await screen.findByTestId('confirm-footer-button')).toBeDisabled(); + }); }); diff --git a/ui/pages/confirmations/components/confirm/footer/footer.test.tsx b/ui/pages/confirmations/components/confirm/footer/footer.test.tsx index 09d1fdf5753b..be52409f9e3f 100644 --- a/ui/pages/confirmations/components/confirm/footer/footer.test.tsx +++ b/ui/pages/confirmations/components/confirm/footer/footer.test.tsx @@ -358,6 +358,24 @@ describe('ConfirmFooter', () => { expect(getByText('Confirm')).toBeInTheDocument(); }); + it('renders the "confirm" button disabled when there are blocking dangerous banner alerts', () => { + const stateWithBannerDangerAlertMock = createStateWithAlerts( + [ + { + ...alertsMock[0], + isBlocking: true, + field: undefined, + }, + ], + { + [KEY_ALERT_KEY_MOCK]: false, + }, + ); + const { getByText } = render(stateWithBannerDangerAlertMock); + expect(getByText('Confirm')).toBeInTheDocument(); + expect(getByText('Confirm')).toBeDisabled(); + }); + it('renders the "confirm" button when there are no alerts', () => { const { getByText } = render(); expect(getByText('Confirm')).toBeInTheDocument(); diff --git a/ui/pages/confirmations/components/confirm/footer/footer.tsx b/ui/pages/confirmations/components/confirm/footer/footer.tsx index a9aea54c03f7..268a879dc7c1 100644 --- a/ui/pages/confirmations/components/confirm/footer/footer.tsx +++ b/ui/pages/confirmations/components/confirm/footer/footer.tsx @@ -96,14 +96,14 @@ const ConfirmButton = ({ useState(false); const { + alerts, hasDangerAlerts, hasUnconfirmedDangerAlerts, - fieldAlerts, hasUnconfirmedFieldDangerAlerts, unconfirmedFieldDangerAlerts, } = useAlerts(alertOwnerId); - const hasDangerBlockingAlerts = fieldAlerts.some( + const hasDangerBlockingAlerts = alerts.some( (alert) => alert.severity === Severity.Danger && alert.isBlocking, ); diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.test.ts b/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.test.ts index d1960827c0b1..440397e508eb 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.test.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.test.ts @@ -14,9 +14,8 @@ import { useSigningOrSubmittingAlerts } from './useSigningOrSubmittingAlerts'; const EXPECTED_ALERT = { isBlocking: true, key: 'signingOrSubmitting', - message: - 'This transaction will only go through once your previous transaction is complete.', - severity: Severity.Warning, + message: 'A previous transaction is still being signed or submitted', + severity: Severity.Danger, }; const ACCOUNT_ADDRESS = '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'; const TRANSACTION_ID_MOCK = '123-456'; diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.ts b/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.ts index 3cb6c45b31e1..585754d21afc 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useSigningOrSubmittingAlerts.ts @@ -32,8 +32,8 @@ export function useSigningOrSubmittingAlerts(): Alert[] { { isBlocking: true, key: 'signingOrSubmitting', - message: t('alertMessageSigningOrSubmitting'), - severity: Severity.Warning, + message: t('isSigningOrSubmitting'), + severity: Severity.Danger, }, ]; }, [isSigningOrSubmitting]); From 7985a8904560899338ff7b284e3c5ad0304c0218 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Mon, 16 Dec 2024 17:59:13 -0330 Subject: [PATCH 6/7] Update changelog for v12.9.2 (#29249) --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b292e7a0285..eda15db00496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ## [12.9.2] +### Changed +- Display the "Amount" row within the advanced view of contract interaction confirmations, and whenever the amount being sent differs from the "You Send" row of the transaction simulation information by more than 5% ([#29131](https://github.com/MetaMask/metamask-extension/pull/29131)) +- Improved phishing detection protections ([#28782](https://github.com/MetaMask/metamask-extension/pull/28782)) + +### Fixed +- Ensure that the correct fallback letter is used for network icons within the token list ([#29121](https://github.com/MetaMask/metamask-extension/pull/29121)) +- Ensure users have to click through a blocking red warning before submitting multiple Smart Transactions while one is already pending ([#29140](https://github.com/MetaMask/metamask-extension/pull/29140)) +- Prevent users from being stuck on an "Invalid string length" error screen, by deleting tokens from their state of the data was invalid because the `decimals` property of the token was `null` ([#29245](https://github.com/MetaMask/metamask-extension/pull/29245)) ## [12.9.1] ### Changed From 4070d3404574f5bf4e792ea14f667cee91b656fc Mon Sep 17 00:00:00 2001 From: Brian Bergeron Date: Mon, 16 Dec 2024 13:57:50 -0800 Subject: [PATCH 7/7] chore(cherry-pick): migration to remove tokens with null decimals (#29251) Cherry picks https://github.com/MetaMask/metamask-extension/pull/29245 to v12.9.2 Co-authored-by: Salim TOUBAL --- app/scripts/migrations/133.1.test.ts | 224 +++++++++++++++++++++++++++ app/scripts/migrations/133.1.ts | 187 ++++++++++++++++++++++ app/scripts/migrations/index.js | 1 + 3 files changed, 412 insertions(+) create mode 100644 app/scripts/migrations/133.1.test.ts create mode 100644 app/scripts/migrations/133.1.ts diff --git a/app/scripts/migrations/133.1.test.ts b/app/scripts/migrations/133.1.test.ts new file mode 100644 index 000000000000..4f1979f31a97 --- /dev/null +++ b/app/scripts/migrations/133.1.test.ts @@ -0,0 +1,224 @@ +import { cloneDeep } from 'lodash'; +import { TokensControllerState } from '@metamask/assets-controllers'; +import { migrate, version } from './133.1'; + +const sentryCaptureExceptionMock = jest.fn(); +const sentryCaptureMessageMock = jest.fn(); + +global.sentry = { + captureException: sentryCaptureExceptionMock, + captureMessage: sentryCaptureMessageMock, +}; + +const oldVersion = 133; + +const mockStateWithNullDecimals = { + meta: { version: oldVersion }, + data: { + TokensController: { + allTokens: { + '0x1': { + '0x123': [ + { address: '0x1', symbol: 'TOKEN1', decimals: null }, + { address: '0x2', symbol: 'TOKEN2', decimals: 18 }, + ], + }, + }, + allDetectedTokens: { + '0x1': { + '0x123': [ + { address: '0x5', symbol: 'TOKEN5', decimals: null }, + { address: '0x6', symbol: 'TOKEN6', decimals: 6 }, + ], + }, + }, + tokens: [ + { address: '0x7', symbol: 'TOKEN7', decimals: null }, + { address: '0x8', symbol: 'TOKEN8', decimals: 18 }, + ], + detectedTokens: [ + { address: '0x9', symbol: 'TOKEN9', decimals: null }, + { address: '0xA', symbol: 'TOKEN10', decimals: 6 }, + ], + }, + }, +}; + +describe(`migration #${version}`, () => { + afterEach(() => jest.resetAllMocks()); + + it('updates the version metadata', async () => { + const oldStorage = cloneDeep(mockStateWithNullDecimals); + + const newStorage = await migrate(oldStorage); + + expect(newStorage.meta).toStrictEqual({ version }); + }); + + it('removes tokens with null decimals from allTokens', async () => { + const oldStorage = cloneDeep(mockStateWithNullDecimals); + + const newStorage = await migrate(oldStorage); + + const tokensControllerState = newStorage.data + .TokensController as TokensControllerState; + const { allTokens } = tokensControllerState; + + expect(allTokens).toEqual({ + '0x1': { + '0x123': [{ address: '0x2', symbol: 'TOKEN2', decimals: 18 }], + }, + }); + }); + + it('removes tokens with null decimals from allDetectedTokens', async () => { + const oldStorage = cloneDeep(mockStateWithNullDecimals); + + const newStorage = await migrate(oldStorage); + + const tokensControllerState = newStorage.data + .TokensController as TokensControllerState; + const { allDetectedTokens } = tokensControllerState; + + expect(allDetectedTokens).toEqual({ + '0x1': { + '0x123': [{ address: '0x6', symbol: 'TOKEN6', decimals: 6 }], + }, + }); + }); + + it('removes tokens with null decimals from tokens array', async () => { + const oldStorage = cloneDeep(mockStateWithNullDecimals); + + const newStorage = await migrate(oldStorage); + + const tokensControllerState = newStorage.data + .TokensController as TokensControllerState; + const { tokens } = tokensControllerState; + + expect(tokens).toEqual([ + { address: '0x8', symbol: 'TOKEN8', decimals: 18 }, + ]); + }); + + it('removes tokens with null decimals from detectedTokens array', async () => { + const oldStorage = cloneDeep(mockStateWithNullDecimals); + + const newStorage = await migrate(oldStorage); + + const tokensControllerState = newStorage.data + .TokensController as TokensControllerState; + const { detectedTokens } = tokensControllerState; + + expect(detectedTokens).toEqual([ + { address: '0xA', symbol: 'TOKEN10', decimals: 6 }, + ]); + }); + + it('logs tokens with null decimals before removing them', async () => { + const oldStorage = cloneDeep(mockStateWithNullDecimals); + + await migrate(oldStorage); + + expect(sentryCaptureMessageMock).toHaveBeenCalledTimes(4); + expect(sentryCaptureMessageMock).toHaveBeenCalledWith( + `Migration ${version}: Removed token with decimals === null in allTokens. Address: 0x1`, + ); + expect(sentryCaptureMessageMock).toHaveBeenCalledWith( + `Migration ${version}: Removed token with decimals === null in allDetectedTokens. Address: 0x5`, + ); + expect(sentryCaptureMessageMock).toHaveBeenCalledWith( + `Migration ${version}: Removed token with decimals === null in tokens. Address: 0x7`, + ); + expect(sentryCaptureMessageMock).toHaveBeenCalledWith( + `Migration ${version}: Removed token with decimals === null in detectedTokens. Address: 0x9`, + ); + }); + + it('does nothing if all tokens have valid decimals', async () => { + const validState = { + meta: { version: oldVersion }, + data: { + TokensController: { + allTokens: { + '0x1': { + '0x123': [{ address: '0x2', symbol: 'TOKEN2', decimals: 18 }], + }, + }, + allDetectedTokens: { + '0x1': { + '0x123': [{ address: '0x6', symbol: 'TOKEN6', decimals: 6 }], + }, + }, + tokens: [{ address: '0x8', symbol: 'TOKEN8', decimals: 18 }], + detectedTokens: [{ address: '0xA', symbol: 'TOKEN10', decimals: 6 }], + }, + }, + }; + + const oldStorage = cloneDeep(validState); + const newStorage = await migrate(oldStorage); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + expect(sentryCaptureMessageMock).not.toHaveBeenCalled(); + }); + + it('does nothing if TokensController is missing', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: {}, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(newStorage.data).toStrictEqual(oldStorage.data); + expect(sentryCaptureMessageMock).not.toHaveBeenCalled(); + }); + + const invalidState = [ + { + errorMessage: `Migration ${version}: Invalid allTokens state of type 'string'`, + label: 'Invalid allTokens', + state: { TokensController: { allTokens: 'invalid' } }, + }, + { + errorMessage: `Migration ${version}: Invalid allDetectedTokens state of type 'string'`, + label: 'Invalid allDetectedTokens', + state: { TokensController: { allDetectedTokens: 'invalid' } }, + }, + { + errorMessage: `Migration ${version}: Invalid tokens state of type 'string'`, + label: 'Invalid tokens', + state: { TokensController: { tokens: 'invalid' } }, + }, + { + errorMessage: `Migration ${version}: Invalid detectedTokens state of type 'string'`, + label: 'Invalid detectedTokens', + state: { TokensController: { detectedTokens: 'invalid' } }, + }, + ]; + + // @ts-expect-error 'each' function is not recognized by TypeScript types + it.each(invalidState)( + 'captures error when state is invalid due to: $label', + async ({ + errorMessage, + state, + }: { + errorMessage: string; + state: Record; + }) => { + const oldStorage = { + meta: { version: oldVersion }, + data: state, + }; + + const newStorage = await migrate(cloneDeep(oldStorage)); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( + new Error(errorMessage), + ); + expect(newStorage.data).toStrictEqual(oldStorage.data); + }, + ); +}); diff --git a/app/scripts/migrations/133.1.ts b/app/scripts/migrations/133.1.ts new file mode 100644 index 000000000000..c59b5c0cfed5 --- /dev/null +++ b/app/scripts/migrations/133.1.ts @@ -0,0 +1,187 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { cloneDeep } from 'lodash'; + +type VersionedData = { + meta: { version: number }; + data: Record; +}; + +export const version = 133.1; + +/** + * Removes tokens with `decimals === null` from `allTokens`, `allDetectedTokens`, `tokens`, and `detectedTokens`. + * Captures exceptions for invalid states using Sentry and logs tokens with `decimals === null`. + * + * @param originalVersionedData - Versioned MetaMask extension state, exactly + * what we persist to disk. + * @returns Updated versioned MetaMask extension state. + */ +export async function migrate( + originalVersionedData: VersionedData, +): Promise { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + transformState(versionedData.data); + return versionedData; +} + +/** + * Transforms the TokensController state to remove tokens with `decimals === null`. + * + * @param state - The persisted MetaMask state. + */ +function transformState(state: Record): void { + if (!hasProperty(state, 'TokensController')) { + return; + } + + const tokensControllerState = state.TokensController; + + if (!isObject(tokensControllerState)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid TokensController state of type '${typeof tokensControllerState}'`, + ), + ); + return; + } + + // Validate and transform `allTokens` + if (hasProperty(tokensControllerState, 'allTokens')) { + if (isObject(tokensControllerState.allTokens)) { + tokensControllerState.allTokens = transformTokenCollection( + tokensControllerState.allTokens, + 'allTokens', + ); + } else { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid allTokens state of type '${typeof tokensControllerState.allTokens}'`, + ), + ); + } + } + + // Validate and transform `allDetectedTokens` + if (hasProperty(tokensControllerState, 'allDetectedTokens')) { + if (isObject(tokensControllerState.allDetectedTokens)) { + tokensControllerState.allDetectedTokens = transformTokenCollection( + tokensControllerState.allDetectedTokens, + 'allDetectedTokens', + ); + } else { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid allDetectedTokens state of type '${typeof tokensControllerState.allDetectedTokens}'`, + ), + ); + } + } + + // Transform `tokens` array + if ( + hasProperty(tokensControllerState, 'tokens') && + Array.isArray(tokensControllerState.tokens) + ) { + tokensControllerState.tokens = tokensControllerState.tokens.filter( + (token) => { + if ( + isObject(token) && + hasProperty(token, 'decimals') && + token.decimals === null && + hasProperty(token, 'address') + ) { + global.sentry?.captureMessage( + `Migration ${version}: Removed token with decimals === null in tokens. Address: ${token.address}`, + ); + return false; + } + return true; + }, + ); + } else if (hasProperty(tokensControllerState, 'tokens')) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid tokens state of type '${typeof tokensControllerState.tokens}'`, + ), + ); + } + + // Transform `detectedTokens` array + if ( + hasProperty(tokensControllerState, 'detectedTokens') && + Array.isArray(tokensControllerState.detectedTokens) + ) { + tokensControllerState.detectedTokens = + tokensControllerState.detectedTokens.filter((token) => { + if ( + isObject(token) && + hasProperty(token, 'decimals') && + token.decimals === null && + hasProperty(token, 'address') + ) { + global.sentry?.captureMessage( + `Migration ${version}: Removed token with decimals === null in detectedTokens. Address: ${token.address}`, + ); + return false; + } + return true; + }); + } else if (hasProperty(tokensControllerState, 'detectedTokens')) { + global.sentry?.captureException( + new Error( + `Migration ${version}: Invalid detectedTokens state of type '${typeof tokensControllerState.detectedTokens}'`, + ), + ); + } +} + +/** + * Removes tokens with `decimals === null` from a token collection and logs their addresses. + * + * @param tokenCollection - The token collection to transform. + * @param propertyName - The name of the property being transformed (for logging purposes). + * @returns The updated token collection. + */ +function transformTokenCollection( + tokenCollection: Record, + propertyName: string, +) { + const updatedState: Record = {}; + + for (const [chainId, accounts] of Object.entries(tokenCollection)) { + if (isObject(accounts)) { + const updatedTokensAccounts: Record = {}; + + for (const [account, tokens] of Object.entries(accounts)) { + if (Array.isArray(tokens)) { + // Filter tokens and log those with `decimals === null` + const filteredTokens = tokens.filter((token) => { + if ( + isObject(token) && + hasProperty(token, 'decimals') && + token.decimals === null && + hasProperty(token, 'address') + ) { + global.sentry?.captureMessage( + `Migration ${version}: Removed token with decimals === null in ${propertyName}. Address: ${token.address}`, + ); + return false; // Exclude token + } + return ( + isObject(token) && + hasProperty(token, 'decimals') && + token.decimals !== null + ); + }); + + updatedTokensAccounts[account] = filteredTokens; + } + } + + updatedState[chainId] = updatedTokensAccounts; + } + } + + return updatedState; +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 08c6eb6dcae6..fbee63b7f7f2 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -155,6 +155,7 @@ const migrations = [ require('./131.1'), require('./132'), require('./133'), + require('./133.1'), ]; export default migrations;