From e9ad9bd42e4eebed5185daa80e2bd056c2e92f68 Mon Sep 17 00:00:00 2001 From: wzrdk3lly Date: Thu, 13 Aug 2026 18:37:18 -0500 Subject: [PATCH 1/3] feat(confirmations): scan EIP-712 address fields via phishing controller Wires the signature address scanning feature into the confirmation flow using `extractSignatureAddresses` from `@metamask/phishing-controller` rather than a local copy of the extractor. After PPOM validates a v3/v4 typed-data signature request and returns a non-malicious result, address-typed fields in the EIP-712 message are extracted and scanned via `PhishingController.scanAddress`. Results are surfaced as alerts via the new `useSignatureAddressAlerts` hook. Note: requires a bump to the `@metamask/phishing-controller` version that exports `extractSignatureAddresses` (pending MetaMask/core PR). Related: #34428 --- .../Views/confirmations/constants/alerts.ts | 3 + .../hooks/alerts/useConfirmationAlerts.ts | 7 +- .../hooks/alerts/useSignatureAddressAlerts.ts | 130 ++++++++++++++++++ .../metrics/useConfirmationAlertMetrics.ts | 6 + .../scan-unvalidated-signature.ts | 55 ++++++++ app/lib/ppom/ppom-util.ts | 13 ++ locales/languages/en.json | 14 ++ 7 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.ts create mode 100644 app/lib/address-scanning/scan-unvalidated-signature.ts diff --git a/app/components/Views/confirmations/constants/alerts.ts b/app/components/Views/confirmations/constants/alerts.ts index 4e90b3c2ecb1..e57db7e5bf44 100644 --- a/app/components/Views/confirmations/constants/alerts.ts +++ b/app/components/Views/confirmations/constants/alerts.ts @@ -30,4 +30,7 @@ export enum AlertKeys { TokenContractAddress = 'token_contract_address', TokenTrustSignalMalicious = 'token_trust_signal_malicious', TokenTrustSignalWarning = 'token_trust_signal_warning', + SignatureAddressScanIncomplete = 'signature_address_scan_incomplete', + SignatureAddressTrustSignalMalicious = 'signature_address_trust_signal_malicious', + SignatureAddressTrustSignalWarning = 'signature_address_trust_signal_warning', } diff --git a/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts b/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts index 2b357a547b61..6bb658a252d6 100644 --- a/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts +++ b/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts @@ -25,11 +25,16 @@ import { useAddressPoisoningAlert } from './useAddressPoisoningAlert'; import { useTokenContractAlert } from './useTokenContractAlert'; import { useAccountNoFundsAlert } from './useAccountNoFundsAlert'; import { useMMPayHardwareAccountAlert } from './useMMPayHardwareAccountAlert'; +import { useSignatureAddressAlerts } from './useSignatureAddressAlerts'; function useSignatureAlerts(): Alert[] { const domainMismatchAlerts = useDomainMismatchAlerts(); + const signatureAddressAlerts = useSignatureAddressAlerts(); - return useMemo(() => [...domainMismatchAlerts], [domainMismatchAlerts]); + return useMemo( + () => [...domainMismatchAlerts, ...signatureAddressAlerts], + [domainMismatchAlerts, signatureAddressAlerts], + ); } function useTransactionAlerts(): Alert[] { diff --git a/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.ts b/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.ts new file mode 100644 index 000000000000..193d76ba91f0 --- /dev/null +++ b/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.ts @@ -0,0 +1,130 @@ +import { useMemo } from 'react'; +import { useSelector } from 'react-redux'; +import { SignTypedDataVersion } from '@metamask/eth-sig-util'; +import { + MessageParamsTyped, + SignatureRequestType, +} from '@metamask/signature-controller'; +import { extractSignatureAddresses } from '@metamask/phishing-controller'; + +import { strings } from '../../../../../../locales/i18n'; +import { renderShortAddress } from '../../../../../util/address'; +import { selectIsSecurityAlertsEnabled } from '../../../../../selectors/preferencesController'; +import { parseTypedDataMessage } from '../../../../../lib/address-scanning/address-scan-util'; +import { Alert, Severity } from '../../types/alerts'; +import { AlertKeys } from '../../constants/alerts'; +import { RowAlertKey } from '../../components/UI/info-row/alert-row/constants'; +import { TrustSignalDisplayState } from '../../types/trustSignals'; +import { useAddressTrustSignals } from '../useAddressTrustSignals'; +import { useSignatureRequest } from '../signatures/useSignatureRequest'; + +/** + * Generate trust-signal alerts for the address fields of a typed-data signature. + */ +export function useSignatureAddressAlerts(): Alert[] { + const signatureRequest = useSignatureRequest(); + const isSecurityAlertsEnabled = useSelector(selectIsSecurityAlertsEnabled); + + const { + addresses: signatureAddresses, + fields, + overflow, + } = useMemo(() => { + const empty = { + addresses: [] as string[], + fields: {} as Record, + overflow: false, + }; + + if (!signatureRequest || !isSecurityAlertsEnabled) { + return empty; + } + + const version = (signatureRequest.messageParams as MessageParamsTyped) + ?.version; + const isTypedSignV3V4 = + signatureRequest.type === SignatureRequestType.TypedSign && + (version === SignTypedDataVersion.V3 || + version === SignTypedDataVersion.V4); + + const msgData = signatureRequest.messageParams?.data; + if (!isTypedSignV3V4 || typeof msgData !== 'string') { + return empty; + } + + const parsed = parseTypedDataMessage(msgData); + if (!parsed) { + return empty; + } + + const signer = signatureRequest.messageParams?.from; + return extractSignatureAddresses(parsed, { + exclude: typeof signer === 'string' ? [signer] : [], + }); + }, [signatureRequest, isSecurityAlertsEnabled]); + + const chainId = signatureRequest?.chainId; + + const trustSignalRequests = useMemo( + () => + chainId + ? signatureAddresses.map((address) => ({ address, chainId })) + : [], + [signatureAddresses, chainId], + ); + + const trustSignals = useAddressTrustSignals(trustSignalRequests); + + return useMemo(() => { + const alerts: Alert[] = []; + + if (overflow) { + alerts.push({ + key: AlertKeys.SignatureAddressScanIncomplete, + field: RowAlertKey.InteractingWith, + severity: Severity.Warning, + message: strings( + 'alert_system.signature_address_scan.incomplete.message', + ), + title: strings('alert_system.signature_address_scan.incomplete.title'), + isBlocking: false, + }); + } + + if (signatureAddresses.length === 0) { + return alerts; + } + + trustSignals.forEach(({ state }, index) => { + const address = signatureAddresses[index]; + + if (state === TrustSignalDisplayState.Malicious) { + alerts.push({ + key: `${AlertKeys.SignatureAddressTrustSignalMalicious}_${address}`, + field: RowAlertKey.InteractingWith, + severity: Severity.Danger, + message: strings( + 'alert_system.signature_address_scan.malicious.message', + { field: fields[address], address: renderShortAddress(address) }, + ), + title: strings('alert_system.signature_address_scan.malicious.title'), + isBlocking: false, + }); + } else if (state === TrustSignalDisplayState.Warning) { + alerts.push({ + key: `${AlertKeys.SignatureAddressTrustSignalWarning}_${address}`, + field: RowAlertKey.InteractingWith, + severity: Severity.Warning, + message: strings( + 'alert_system.signature_address_scan.warning.message', + { field: fields[address], address: renderShortAddress(address) }, + ), + title: strings('alert_system.signature_address_scan.warning.title'), + isBlocking: false, + }); + } + }); + + return alerts; + }, [signatureAddresses, fields, overflow, trustSignals]); +} diff --git a/app/components/Views/confirmations/hooks/metrics/useConfirmationAlertMetrics.ts b/app/components/Views/confirmations/hooks/metrics/useConfirmationAlertMetrics.ts index 58d14d028fff..ec6e87e94d17 100644 --- a/app/components/Views/confirmations/hooks/metrics/useConfirmationAlertMetrics.ts +++ b/app/components/Views/confirmations/hooks/metrics/useConfirmationAlertMetrics.ts @@ -137,6 +137,12 @@ const ALERTS_NAME_METRICS: AlertNameMetrics = { [AlertKeys.TokenTrustSignalWarning]: 'token_trust_signal_warning', [AlertKeys.FiatBuyAmountLimit]: 'fiat_buy_amount_limit', [AlertKeys.DepositLimit]: 'deposit_limit', + [AlertKeys.SignatureAddressScanIncomplete]: + 'signature_address_scan_incomplete', + [AlertKeys.SignatureAddressTrustSignalMalicious]: + 'signature_address_trust_signal_malicious', + [AlertKeys.SignatureAddressTrustSignalWarning]: + 'signature_address_trust_signal_warning', }; function getAlertName(alertKey: string): string { diff --git a/app/lib/address-scanning/scan-unvalidated-signature.ts b/app/lib/address-scanning/scan-unvalidated-signature.ts new file mode 100644 index 000000000000..f578ab10fde6 --- /dev/null +++ b/app/lib/address-scanning/scan-unvalidated-signature.ts @@ -0,0 +1,55 @@ +import { + extractSignatureAddresses, + type PhishingController, +} from '@metamask/phishing-controller'; +import { isBlockaidPreferenceEnabled } from '../../util/blockaid'; +import { parseTypedDataMessage, scanAddress } from './address-scan-util'; + +const METHOD_SIGN_TYPED_DATA_V3 = 'eth_signTypedData_v3'; +const METHOD_SIGN_TYPED_DATA_V4 = 'eth_signTypedData_v4'; + +/** + * Scan the address fields of a typed-data signature request. + */ +export function scanUnvalidatedSignatureAddresses({ + request, + chainId, + phishingController, +}: { + request: { method: string; params?: unknown }; + chainId: string; + phishingController: PhishingController; +}): void { + if ( + request.method !== METHOD_SIGN_TYPED_DATA_V3 && + request.method !== METHOD_SIGN_TYPED_DATA_V4 + ) { + return; + } + + if (!isBlockaidPreferenceEnabled()) { + return; + } + + const { params } = request; + if (!Array.isArray(params) || params[1] === undefined || params[1] === null) { + return; + } + + const typedDataMessage = parseTypedDataMessage( + typeof params[1] === 'string' ? params[1] : JSON.stringify(params[1]), + ); + if (!typedDataMessage) { + return; + } + + const signerAddress = typeof params[0] === 'string' ? params[0] : undefined; + + const { addresses } = extractSignatureAddresses(typedDataMessage, { + exclude: signerAddress ? [signerAddress] : [], + }); + + for (const address of addresses) { + scanAddress(phishingController, chainId, address); + } +} diff --git a/app/lib/ppom/ppom-util.ts b/app/lib/ppom/ppom-util.ts index c840d5d9064c..5745060c97b1 100644 --- a/app/lib/ppom/ppom-util.ts +++ b/app/lib/ppom/ppom-util.ts @@ -23,6 +23,7 @@ import { import { WALLET_CONNECT_ORIGIN } from '../../util/walletconnect'; import AppConstants from '../../core/AppConstants'; import { validateWithSecurityAlertsAPI } from './security-alerts-api'; +import { scanUnvalidatedSignatureAddresses } from '../address-scanning/scan-unvalidated-signature'; import { Messenger } from '@metamask/messenger'; import { SignatureStateChange } from '@metamask/signature-controller'; import cloneDeep from 'lodash/cloneDeep'; @@ -167,6 +168,18 @@ async function validateRequest( updateControllerState: true, securityAlertId, }); + + const resultType = securityAlertResponse?.result_type; + if ( + resultType !== ResultType.Malicious && + resultType !== ResultType.Warning + ) { + scanUnvalidatedSignatureAddresses({ + request: { method, params: req.params }, + chainId, + phishingController: Engine.context.PhishingController, + }); + } } } diff --git a/locales/languages/en.json b/locales/languages/en.json index 5b115b274bbc..39c92e49ea49 100644 --- a/locales/languages/en.json +++ b/locales/languages/en.json @@ -185,6 +185,20 @@ "message": "This has been identified as suspicious. We recommend not interacting with this site.", "title": "Suspicious site" } + }, + "signature_address_scan": { + "malicious": { + "message": "The {{field}} address {{address}} has been identified as malicious.", + "title": "Malicious address" + }, + "warning": { + "message": "The {{field}} address {{address}} needs review. Only continue if you trust the source.", + "title": "Address needs review" + }, + "incomplete": { + "message": "Some addresses in this request couldn't be checked for known threats. Only continue if you trust this site.", + "title": "Addresses not fully checked" + } } }, "blockaid_banner": { From b1dd2ccf5f1952fc3784951e3e7ee706caf9ad93 Mon Sep 17 00:00:00 2001 From: wzrdk3lly Date: Thu, 13 Aug 2026 19:31:47 -0500 Subject: [PATCH 2/3] test(confirmations): add unit tests for signature address scan and hook --- .../alerts/useSignatureAddressAlerts.test.ts | 317 ++++++++++++++++++ .../scan-unvalidated-signature.test.ts | 220 ++++++++++++ 2 files changed, 537 insertions(+) create mode 100644 app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.test.ts create mode 100644 app/lib/address-scanning/scan-unvalidated-signature.test.ts diff --git a/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.test.ts b/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.test.ts new file mode 100644 index 000000000000..98d4f328ffe9 --- /dev/null +++ b/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.test.ts @@ -0,0 +1,317 @@ +import { + SignatureRequest, + SignatureRequestType, +} from '@metamask/signature-controller'; +import { SignTypedDataVersion } from '@metamask/eth-sig-util'; + +import { renderHookWithProvider } from '../../../../../util/test/renderWithProvider'; +import { RowAlertKey } from '../../components/UI/info-row/alert-row/constants'; +import { AlertKeys } from '../../constants/alerts'; +import { Severity } from '../../types/alerts'; +import { TrustSignalDisplayState } from '../../types/trustSignals'; +import { useSignatureAddressAlerts } from './useSignatureAddressAlerts'; +import { useSignatureRequest } from '../signatures/useSignatureRequest'; +import { useAddressTrustSignals } from '../useAddressTrustSignals'; +import { parseTypedDataMessage } from '../../../../../lib/address-scanning/address-scan-util'; + +jest.mock('../signatures/useSignatureRequest', () => ({ + useSignatureRequest: jest.fn(), +})); + +jest.mock('../useAddressTrustSignals', () => ({ + useAddressTrustSignals: jest.fn(), +})); + +jest.mock('../../../../../lib/address-scanning/address-scan-util', () => ({ + ...jest.requireActual( + '../../../../../lib/address-scanning/address-scan-util', + ), + parseTypedDataMessage: jest.fn(), +})); + +const mockUseSignatureRequest = jest.mocked(useSignatureRequest); +const mockUseAddressTrustSignals = jest.mocked(useAddressTrustSignals); +const mockParseTypedDataMessage = jest.mocked(parseTypedDataMessage); + +const MALICIOUS_ADDRESS = '0x0000000000000000000000000000000000000bad'; +const WARNING_ADDRESS = '0x0000000000000000000000000000000000000001'; +const SIGNER_ADDRESS = '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; +const CHAIN_ID = '0x1'; + +const enabledState = { + state: { + engine: { + backgroundState: { + PreferencesController: { + securityAlertsEnabled: true, + }, + }, + }, + }, +}; + +const makeTypedSignRequest = ( + data: object, + version: SignTypedDataVersion = SignTypedDataVersion.V4, +): SignatureRequest => + ({ + type: SignatureRequestType.TypedSign, + chainId: CHAIN_ID, + messageParams: { + from: SIGNER_ADDRESS, + data: JSON.stringify(data), + version, + }, + }) as unknown as SignatureRequest; + +const SIMPLE_TYPED_DATA = { + types: { + Transfer: [{ name: 'recipient', type: 'address' }], + }, + primaryType: 'Transfer', + message: { recipient: MALICIOUS_ADDRESS }, +}; + +describe('useSignatureAddressAlerts (mobile)', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockUseAddressTrustSignals.mockReturnValue([]); + mockParseTypedDataMessage.mockReturnValue(null); + }); + + it('returns empty array when security alerts are disabled', () => { + mockUseSignatureRequest.mockReturnValue( + makeTypedSignRequest(SIMPLE_TYPED_DATA), + ); + mockParseTypedDataMessage.mockReturnValue(SIMPLE_TYPED_DATA); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + { + state: { + engine: { + backgroundState: { + PreferencesController: { securityAlertsEnabled: false }, + }, + }, + }, + }, + ); + + expect(result.current).toEqual([]); + expect(mockUseAddressTrustSignals).toHaveBeenCalledWith([]); + }); + + it('returns empty array when there is no signature request', () => { + mockUseSignatureRequest.mockReturnValue(undefined); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect(result.current).toEqual([]); + }); + + it('returns empty array for non-typed-sign request type', () => { + mockUseSignatureRequest.mockReturnValue({ + type: SignatureRequestType.PersonalSign, + chainId: CHAIN_ID, + messageParams: { from: SIGNER_ADDRESS, data: 'hello' }, + } as unknown as SignatureRequest); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect(result.current).toEqual([]); + expect(mockUseAddressTrustSignals).toHaveBeenCalledWith([]); + }); + + it('returns empty array for v1 typed data', () => { + mockUseSignatureRequest.mockReturnValue( + makeTypedSignRequest(SIMPLE_TYPED_DATA, SignTypedDataVersion.V1), + ); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect(result.current).toEqual([]); + expect(mockUseAddressTrustSignals).toHaveBeenCalledWith([]); + }); + + it('returns empty array when parseTypedDataMessage returns null', () => { + mockUseSignatureRequest.mockReturnValue( + makeTypedSignRequest(SIMPLE_TYPED_DATA), + ); + mockParseTypedDataMessage.mockReturnValue(null); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect(result.current).toEqual([]); + }); + + it('returns empty array when no addresses in message', () => { + const typedData = { + types: { Greeting: [{ name: 'text', type: 'string' }] }, + primaryType: 'Greeting', + message: { text: 'Hello world' }, + }; + mockUseSignatureRequest.mockReturnValue(makeTypedSignRequest(typedData)); + mockParseTypedDataMessage.mockReturnValue(typedData); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect(result.current).toEqual([]); + }); + + it('returns danger alert for malicious address field', () => { + mockUseSignatureRequest.mockReturnValue( + makeTypedSignRequest(SIMPLE_TYPED_DATA), + ); + mockParseTypedDataMessage.mockReturnValue(SIMPLE_TYPED_DATA); + mockUseAddressTrustSignals.mockReturnValue([ + { state: TrustSignalDisplayState.Malicious, label: null }, + ]); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect(result.current).toHaveLength(1); + expect(result.current[0]).toMatchObject({ + key: `${AlertKeys.SignatureAddressTrustSignalMalicious}_${MALICIOUS_ADDRESS}`, + field: RowAlertKey.InteractingWith, + severity: Severity.Danger, + isBlocking: false, + }); + }); + + it('returns warning alert for flagged address field', () => { + const typedData = { + types: { T: [{ name: 'addr', type: 'address' }] }, + primaryType: 'T', + message: { addr: WARNING_ADDRESS }, + }; + mockUseSignatureRequest.mockReturnValue(makeTypedSignRequest(typedData)); + mockParseTypedDataMessage.mockReturnValue(typedData); + mockUseAddressTrustSignals.mockReturnValue([ + { state: TrustSignalDisplayState.Warning, label: null }, + ]); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect(result.current).toHaveLength(1); + expect(result.current[0]).toMatchObject({ + key: `${AlertKeys.SignatureAddressTrustSignalWarning}_${WARNING_ADDRESS}`, + field: RowAlertKey.InteractingWith, + severity: Severity.Warning, + }); + }); + + it('returns no alerts for Unknown trust signal state', () => { + mockUseSignatureRequest.mockReturnValue( + makeTypedSignRequest(SIMPLE_TYPED_DATA), + ); + mockParseTypedDataMessage.mockReturnValue(SIMPLE_TYPED_DATA); + mockUseAddressTrustSignals.mockReturnValue([ + { state: TrustSignalDisplayState.Unknown, label: null }, + ]); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect(result.current).toEqual([]); + }); + + it('excludes the signer address', () => { + const typedData = { + types: { T: [{ name: 'addr', type: 'address' }] }, + primaryType: 'T', + message: { addr: SIGNER_ADDRESS }, + }; + mockUseSignatureRequest.mockReturnValue(makeTypedSignRequest(typedData)); + mockParseTypedDataMessage.mockReturnValue(typedData); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect(mockUseAddressTrustSignals).toHaveBeenCalledWith([]); + expect(result.current).toEqual([]); + }); + + it('returns overflow caution alert when address cap is exceeded', () => { + const types: { name: string; type: string }[] = []; + const message: Record = {}; + for (let i = 0; i < 12; i += 1) { + types.push({ name: `addr${i}`, type: 'address' }); + message[`addr${i}`] = `0x${String(i).padStart(40, '0')}`; + } + const typedData = { + types: { Flood: types }, + primaryType: 'Flood', + message, + }; + + mockUseSignatureRequest.mockReturnValue(makeTypedSignRequest(typedData)); + mockParseTypedDataMessage.mockReturnValue(typedData); + mockUseAddressTrustSignals.mockReturnValue( + new Array(10).fill({ + state: TrustSignalDisplayState.Unknown, + label: null, + }), + ); + + const { result } = renderHookWithProvider( + () => useSignatureAddressAlerts(), + enabledState, + ); + + expect( + result.current.some( + (a) => a.key === AlertKeys.SignatureAddressScanIncomplete, + ), + ).toBe(true); + expect( + result.current.find( + (a) => a.key === AlertKeys.SignatureAddressScanIncomplete, + ), + ).toMatchObject({ + field: RowAlertKey.InteractingWith, + severity: Severity.Warning, + }); + }); + + it('passes address+chainId pairs to useAddressTrustSignals', () => { + mockUseSignatureRequest.mockReturnValue( + makeTypedSignRequest(SIMPLE_TYPED_DATA), + ); + mockParseTypedDataMessage.mockReturnValue(SIMPLE_TYPED_DATA); + mockUseAddressTrustSignals.mockReturnValue([ + { state: TrustSignalDisplayState.Unknown, label: null }, + ]); + + renderHookWithProvider(() => useSignatureAddressAlerts(), enabledState); + + expect(mockUseAddressTrustSignals).toHaveBeenCalledWith([ + { address: MALICIOUS_ADDRESS, chainId: CHAIN_ID }, + ]); + }); +}); diff --git a/app/lib/address-scanning/scan-unvalidated-signature.test.ts b/app/lib/address-scanning/scan-unvalidated-signature.test.ts new file mode 100644 index 000000000000..8d7734c063d6 --- /dev/null +++ b/app/lib/address-scanning/scan-unvalidated-signature.test.ts @@ -0,0 +1,220 @@ +import { scanUnvalidatedSignatureAddresses } from './scan-unvalidated-signature'; + +const MALICIOUS_ADDRESS = '0x0000000000000000000000000000000000000bad'; +const SIGNER_ADDRESS = '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; +const CHAIN_ID = '0x1'; + +const TYPED_DATA_V4 = { + types: { + Transfer: [{ name: 'recipient', type: 'address' }], + }, + primaryType: 'Transfer', + message: { recipient: MALICIOUS_ADDRESS }, +}; + +jest.mock('../../util/blockaid', () => ({ + isBlockaidPreferenceEnabled: jest.fn(), +})); + +jest.mock('./address-scan-util', () => ({ + parseTypedDataMessage: jest.fn(), + scanAddress: jest.fn(), +})); + +const mockIsBlockaidPreferenceEnabled = jest.requireMock( + '../../util/blockaid', +).isBlockaidPreferenceEnabled; + +const mockParseTypedDataMessage = jest.requireMock( + './address-scan-util', +).parseTypedDataMessage; + +const mockScanAddress = jest.requireMock('./address-scan-util').scanAddress; + +describe('scanUnvalidatedSignatureAddresses (mobile)', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockIsBlockaidPreferenceEnabled.mockReturnValue(true); + mockParseTypedDataMessage.mockReturnValue(TYPED_DATA_V4); + }); + + const makeRequest = ( + method: string, + signer: string, + data: unknown, + ): { method: string; params: unknown[] } => ({ + method, + params: [signer, typeof data === 'string' ? data : JSON.stringify(data)], + }); + + it('scans extracted address fields for v4 typed data', () => { + const phishingController = {} as never; + scanUnvalidatedSignatureAddresses({ + request: makeRequest( + 'eth_signTypedData_v4', + SIGNER_ADDRESS, + TYPED_DATA_V4, + ), + chainId: CHAIN_ID, + phishingController, + }); + + expect(mockScanAddress).toHaveBeenCalledWith( + phishingController, + CHAIN_ID, + MALICIOUS_ADDRESS, + ); + }); + + it('scans extracted address fields for v3 typed data', () => { + const phishingController = {} as never; + scanUnvalidatedSignatureAddresses({ + request: makeRequest( + 'eth_signTypedData_v3', + SIGNER_ADDRESS, + TYPED_DATA_V4, + ), + chainId: CHAIN_ID, + phishingController, + }); + + expect(mockScanAddress).toHaveBeenCalledWith( + phishingController, + CHAIN_ID, + MALICIOUS_ADDRESS, + ); + }); + + it('does nothing for non-typed-data methods', () => { + scanUnvalidatedSignatureAddresses({ + request: { + method: 'personal_sign', + params: [SIGNER_ADDRESS, '0xdeadbeef'], + }, + chainId: CHAIN_ID, + phishingController: {} as never, + }); + + expect(mockScanAddress).not.toHaveBeenCalled(); + }); + + it('does nothing for v1 typed data method', () => { + scanUnvalidatedSignatureAddresses({ + request: { method: 'eth_signTypedData', params: [SIGNER_ADDRESS, '{}'] }, + chainId: CHAIN_ID, + phishingController: {} as never, + }); + + expect(mockScanAddress).not.toHaveBeenCalled(); + }); + + it('does nothing when Blockaid preference is disabled', () => { + mockIsBlockaidPreferenceEnabled.mockReturnValue(false); + + scanUnvalidatedSignatureAddresses({ + request: makeRequest( + 'eth_signTypedData_v4', + SIGNER_ADDRESS, + TYPED_DATA_V4, + ), + chainId: CHAIN_ID, + phishingController: {} as never, + }); + + expect(mockScanAddress).not.toHaveBeenCalled(); + }); + + it('does nothing when params are missing', () => { + scanUnvalidatedSignatureAddresses({ + request: { method: 'eth_signTypedData_v4' }, + chainId: CHAIN_ID, + phishingController: {} as never, + }); + + expect(mockScanAddress).not.toHaveBeenCalled(); + }); + + it('does nothing when parseTypedDataMessage returns null', () => { + mockParseTypedDataMessage.mockReturnValue(null); + + scanUnvalidatedSignatureAddresses({ + request: makeRequest( + 'eth_signTypedData_v4', + SIGNER_ADDRESS, + TYPED_DATA_V4, + ), + chainId: CHAIN_ID, + phishingController: {} as never, + }); + + expect(mockScanAddress).not.toHaveBeenCalled(); + }); + + it('excludes the signer address', () => { + mockParseTypedDataMessage.mockReturnValue({ + types: { T: [{ name: 'addr', type: 'address' }] }, + primaryType: 'T', + message: { addr: SIGNER_ADDRESS }, + }); + + scanUnvalidatedSignatureAddresses({ + request: makeRequest('eth_signTypedData_v4', SIGNER_ADDRESS, {}), + chainId: CHAIN_ID, + phishingController: {} as never, + }); + + expect(mockScanAddress).not.toHaveBeenCalled(); + }); + + it('accepts typed data as an object in params[1]', () => { + const phishingController = {} as never; + scanUnvalidatedSignatureAddresses({ + request: { + method: 'eth_signTypedData_v4', + params: [SIGNER_ADDRESS, TYPED_DATA_V4], + }, + chainId: CHAIN_ID, + phishingController, + }); + + expect(mockScanAddress).toHaveBeenCalledWith( + phishingController, + CHAIN_ID, + MALICIOUS_ADDRESS, + ); + }); + + it('scans multiple addresses from nested message types', () => { + const addr1 = '0x0000000000000000000000000000000000000001'; + const addr2 = '0x0000000000000000000000000000000000000002'; + mockParseTypedDataMessage.mockReturnValue({ + types: { + Pair: [ + { name: 'a', type: 'address' }, + { name: 'b', type: 'address' }, + ], + }, + primaryType: 'Pair', + message: { a: addr1, b: addr2 }, + }); + + const phishingController = {} as never; + scanUnvalidatedSignatureAddresses({ + request: makeRequest('eth_signTypedData_v4', SIGNER_ADDRESS, {}), + chainId: CHAIN_ID, + phishingController, + }); + + expect(mockScanAddress).toHaveBeenCalledTimes(2); + expect(mockScanAddress).toHaveBeenCalledWith( + phishingController, + CHAIN_ID, + addr1, + ); + expect(mockScanAddress).toHaveBeenCalledWith( + phishingController, + CHAIN_ID, + addr2, + ); + }); +}); From 098fd998e302960731ca79cb600194fc25597d6e Mon Sep 17 00:00:00 2001 From: wzrdk3lly Date: Thu, 20 Aug 2026 09:48:25 -0500 Subject: [PATCH 3/3] refactor(confirmations): rename hook to useSignatureTrustSignalAlerts and extract baseAlert --- .../hooks/alerts/useConfirmationAlerts.ts | 4 +-- .../alerts/useSignatureAddressAlerts.test.ts | 26 +++++++++---------- .../hooks/alerts/useSignatureAddressAlerts.ts | 19 +++++++++----- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts b/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts index 6bb658a252d6..521d80ff2f7d 100644 --- a/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts +++ b/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts @@ -25,11 +25,11 @@ import { useAddressPoisoningAlert } from './useAddressPoisoningAlert'; import { useTokenContractAlert } from './useTokenContractAlert'; import { useAccountNoFundsAlert } from './useAccountNoFundsAlert'; import { useMMPayHardwareAccountAlert } from './useMMPayHardwareAccountAlert'; -import { useSignatureAddressAlerts } from './useSignatureAddressAlerts'; +import { useSignatureTrustSignalAlerts } from './useSignatureAddressAlerts'; function useSignatureAlerts(): Alert[] { const domainMismatchAlerts = useDomainMismatchAlerts(); - const signatureAddressAlerts = useSignatureAddressAlerts(); + const signatureAddressAlerts = useSignatureTrustSignalAlerts(); return useMemo( () => [...domainMismatchAlerts, ...signatureAddressAlerts], diff --git a/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.test.ts b/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.test.ts index 98d4f328ffe9..f54cb2a270ef 100644 --- a/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.test.ts +++ b/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.test.ts @@ -9,7 +9,7 @@ import { RowAlertKey } from '../../components/UI/info-row/alert-row/constants'; import { AlertKeys } from '../../constants/alerts'; import { Severity } from '../../types/alerts'; import { TrustSignalDisplayState } from '../../types/trustSignals'; -import { useSignatureAddressAlerts } from './useSignatureAddressAlerts'; +import { useSignatureTrustSignalAlerts } from './useSignatureAddressAlerts'; import { useSignatureRequest } from '../signatures/useSignatureRequest'; import { useAddressTrustSignals } from '../useAddressTrustSignals'; import { parseTypedDataMessage } from '../../../../../lib/address-scanning/address-scan-util'; @@ -86,7 +86,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { mockParseTypedDataMessage.mockReturnValue(SIMPLE_TYPED_DATA); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), { state: { engine: { @@ -106,7 +106,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { mockUseSignatureRequest.mockReturnValue(undefined); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -121,7 +121,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { } as unknown as SignatureRequest); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -135,7 +135,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { ); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -150,7 +150,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { mockParseTypedDataMessage.mockReturnValue(null); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -167,7 +167,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { mockParseTypedDataMessage.mockReturnValue(typedData); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -184,7 +184,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { ]); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -210,7 +210,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { ]); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -232,7 +232,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { ]); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -249,7 +249,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { mockParseTypedDataMessage.mockReturnValue(typedData); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -280,7 +280,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { ); const { result } = renderHookWithProvider( - () => useSignatureAddressAlerts(), + () => useSignatureTrustSignalAlerts(), enabledState, ); @@ -308,7 +308,7 @@ describe('useSignatureAddressAlerts (mobile)', () => { { state: TrustSignalDisplayState.Unknown, label: null }, ]); - renderHookWithProvider(() => useSignatureAddressAlerts(), enabledState); + renderHookWithProvider(() => useSignatureTrustSignalAlerts(), enabledState); expect(mockUseAddressTrustSignals).toHaveBeenCalledWith([ { address: MALICIOUS_ADDRESS, chainId: CHAIN_ID }, diff --git a/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.ts b/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.ts index 193d76ba91f0..e995d48ac054 100644 --- a/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.ts +++ b/app/components/Views/confirmations/hooks/alerts/useSignatureAddressAlerts.ts @@ -21,7 +21,7 @@ import { useSignatureRequest } from '../signatures/useSignatureRequest'; /** * Generate trust-signal alerts for the address fields of a typed-data signature. */ -export function useSignatureAddressAlerts(): Alert[] { +export function useSignatureTrustSignalAlerts(): Alert[] { const signatureRequest = useSignatureRequest(); const isSecurityAlertsEnabled = useSelector(selectIsSecurityAlertsEnabled); @@ -95,32 +95,37 @@ export function useSignatureAddressAlerts(): Alert[] { return alerts; } + const baseAlert = { + field: RowAlertKey.InteractingWith, + isBlocking: false as const, + }; + trustSignals.forEach(({ state }, index) => { const address = signatureAddresses[index]; + const shortAddress = renderShortAddress(address); + const fieldLabel = fields[address]; if (state === TrustSignalDisplayState.Malicious) { alerts.push({ + ...baseAlert, key: `${AlertKeys.SignatureAddressTrustSignalMalicious}_${address}`, - field: RowAlertKey.InteractingWith, severity: Severity.Danger, message: strings( 'alert_system.signature_address_scan.malicious.message', - { field: fields[address], address: renderShortAddress(address) }, + { field: fieldLabel, address: shortAddress }, ), title: strings('alert_system.signature_address_scan.malicious.title'), - isBlocking: false, }); } else if (state === TrustSignalDisplayState.Warning) { alerts.push({ + ...baseAlert, key: `${AlertKeys.SignatureAddressTrustSignalWarning}_${address}`, - field: RowAlertKey.InteractingWith, severity: Severity.Warning, message: strings( 'alert_system.signature_address_scan.warning.message', - { field: fields[address], address: renderShortAddress(address) }, + { field: fieldLabel, address: shortAddress }, ), title: strings('alert_system.signature_address_scan.warning.title'), - isBlocking: false, }); } });