diff --git a/packages/extension-base/src/background/RequestBytesSign.ts b/packages/extension-base/src/background/RequestBytesSign.ts index f732672c..1f9c5a24 100644 --- a/packages/extension-base/src/background/RequestBytesSign.ts +++ b/packages/extension-base/src/background/RequestBytesSign.ts @@ -5,11 +5,12 @@ import type { KeyringPair } from '@polkadot/keyring/types'; import type { TypeRegistry } from '@polkadot/types'; import type { SignerPayloadRaw } from '@polkadot/types/types'; import type { HexString } from '@polkadot/util/types'; -import type { RequestSign } from './types.js'; +import type { RequestSignBytes } from './types.js'; import { u8aToHex, u8aWrapBytes } from '@polkadot/util'; -export default class RequestBytesSign implements RequestSign { +export default class RequestBytesSign implements RequestSignBytes { + public readonly channel = 'bytes' as const; public readonly payload: SignerPayloadRaw; constructor (payload: SignerPayloadRaw) { diff --git a/packages/extension-base/src/background/RequestExtrinsicSign.ts b/packages/extension-base/src/background/RequestExtrinsicSign.ts index 156fed88..a36f00d0 100644 --- a/packages/extension-base/src/background/RequestExtrinsicSign.ts +++ b/packages/extension-base/src/background/RequestExtrinsicSign.ts @@ -5,9 +5,10 @@ import type { KeyringPair } from '@polkadot/keyring/types'; import type { TypeRegistry } from '@polkadot/types'; import type { SignerPayloadJSON } from '@polkadot/types/types'; import type { HexString } from '@polkadot/util/types'; -import type { RequestSign } from './types.js'; +import type { RequestSignExtrinsic } from './types.js'; -export default class RequestExtrinsicSign implements RequestSign { +export default class RequestExtrinsicSign implements RequestSignExtrinsic { + public readonly channel = 'extrinsic' as const; public readonly payload: SignerPayloadJSON; constructor (payload: SignerPayloadJSON) { diff --git a/packages/extension-base/src/background/handlers/Extension.spec.ts b/packages/extension-base/src/background/handlers/Extension.spec.ts index d13f286b..68b759cb 100644 --- a/packages/extension-base/src/background/handlers/Extension.spec.ts +++ b/packages/extension-base/src/background/handlers/Extension.spec.ts @@ -15,6 +15,7 @@ import type { KeypairType } from '@polkadot/util-crypto/types'; import { TypeRegistry } from '@polkadot/types'; import keyring from '@polkadot/ui-keyring'; +import { stringToHex } from '@polkadot/util'; import { cryptoWaitReady } from '@polkadot/util-crypto'; import { AccountsStore } from '../../stores/index.js'; @@ -475,4 +476,78 @@ describe('Extension', () => { expect(res).toEqual(true); }); }); + + describe('signer channel', () => { + let address: string, payload: SignerPayloadJSON; + + beforeEach(async () => { + address = await createAccount(); + payload = { + address, + blockHash: '0xe1b1dda72998846487e4d858909d4f9a6bbd6e338e4588e5d809de16b1317b80', + blockNumber: '0x00000393', + era: '0x3601', + genesisHash: '0x242a54b35e1aad38f37b884eddeb71f6f9931b02fac27bf52dfb62ef754e5e62', + method: '0x040105fa8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a4882380100', + nonce: '0x0000000000000000', + signedExtensions: ['CheckSpecVersion', 'CheckTxVersion', 'CheckGenesis', 'CheckMortality', 'CheckNonce', 'CheckWeight', 'ChargeTransactionPayment'], + specVersion: '0x00000026', + tip: '0x00000000000000000000000000000000', + transactionVersion: '0x00000005', + version: 4 + }; + }); + + // `data` has no meaning on the extrinsic channel and does not affect what is + // signed, so it must not reach the UI and steer how the request is shown. + it('rejects a signPayload request carrying raw data', async () => { + const { blockNumber: _omitted, ...rest } = payload; + + await expect(tabs.handle('1615191860871.6', 'pub(extrinsic.sign)', { + ...rest, + data: stringToHex('Sign in to EvilDapp') + } as unknown as SignerPayloadJSON, 'http://localhost:3000', {} as chrome.runtime.Port)) + .rejects.toThrow(/Unexpected raw data/); + }); + + // a falsy `data` never selected the message view, so it must keep signing + it('accepts a signPayload request with a falsy data field', async () => { + // eslint-disable-next-line jest/valid-expect-in-promise + tabs.handle('1615191860871.7', 'pub(extrinsic.sign)', { + ...payload, + data: null + } as unknown as SignerPayloadJSON, 'http://localhost:3000', {} as chrome.runtime.Port) + .catch((err) => console.log(err)); + + const queued = state.allSignRequests[state.allSignRequests.length - 1]; + + const res = await extension.handle('1615192062290.7', 'pri(signing.approve.password)', { + id: queued.id, + password, + savePass: false + }, {} as chrome.runtime.Port); + + expect(res).toEqual(true); + }); + + // The UI reads this instead of sniffing the payload, so a request cannot be + // displayed as one kind and signed as the other. + it('tags a request with the channel it arrived on', async () => { + // eslint-disable-next-line jest/valid-expect-in-promise + tabs.handle('1615191860871.8', 'pub(extrinsic.sign)', payload, 'http://localhost:3000', {} as chrome.runtime.Port) + .catch((err) => console.log(err)); + + const queued = state.allSignRequests[state.allSignRequests.length - 1]; + + expect(queued.request.channel).toEqual('extrinsic'); + + const res = await extension.handle('1615192062290.8', 'pri(signing.approve.password)', { + id: queued.id, + password, + savePass: false + }, {} as chrome.runtime.Port); + + expect(res).toEqual(true); + }); + }); }); diff --git a/packages/extension-base/src/background/handlers/Extension.ts b/packages/extension-base/src/background/handlers/Extension.ts index ccbd9f6e..0d0e34c2 100644 --- a/packages/extension-base/src/background/handlers/Extension.ts +++ b/packages/extension-base/src/background/handlers/Extension.ts @@ -5,7 +5,7 @@ import type { MetadataDef } from '@polkadot/extension-inject/types'; import type { KeyringPair, KeyringPair$Json, KeyringPair$Meta } from '@polkadot/keyring/types'; -import type { Registry, SignerPayloadJSON, SignerPayloadRaw } from '@polkadot/types/types'; +import type { Registry } from '@polkadot/types/types'; import type { SubjectInfo } from '@polkadot/ui-keyring/observable/types'; import type { KeypairType } from '@polkadot/util-crypto/types'; import type { AccountJson, AllowedPath, AuthorizeRequest, MessageTypes, MetadataRequest, RequestAccountBatchExport, RequestAccountChangePassword, RequestAccountCreateExternal, RequestAccountCreateHardware, RequestAccountCreateSuri, RequestAccountEdit, RequestAccountExport, RequestAccountForget, RequestAccountShow, RequestAccountTie, RequestAccountValidate, RequestActiveTabsUrlUpdate, RequestAuthorizeApprove, RequestBatchRestore, RequestDeriveCreate, RequestDeriveValidate, RequestJsonRestore, RequestMetadataApprove, RequestMetadataReject, RequestSeedCreate, RequestSeedValidate, RequestSigningApprovePassword, RequestSigningApproveSignature, RequestSigningCancel, RequestSigningIsLocked, RequestTypes, RequestUpdateAuthorizedAccounts, ResponseAccountExport, ResponseAccountsExport, ResponseAuthorizeList, ResponseDeriveValidate, ResponseJsonGetAccountInfo, ResponseSeedCreate, ResponseSeedValidate, ResponseSigningIsLocked, ResponseType, SigningRequest } from '../types.js'; @@ -20,6 +20,7 @@ import { accounts as accountsObservable } from '@polkadot/ui-keyring/observable/ import { assert, isHex } from '@polkadot/util'; import { keyExtractSuri, mnemonicGenerate, mnemonicValidate } from '@polkadot/util-crypto'; +import { isExtrinsicRequest } from '../../utils/index.js'; import { withErrorLog } from './helpers.js'; import { createSubscription, unsubscribe } from './subscriptions.js'; @@ -35,10 +36,6 @@ function getSuri (seed: string, type?: KeypairType): string { : seed; } -function isJsonPayload (value: SignerPayloadJSON | SignerPayloadRaw): value is SignerPayloadJSON { - return (value as SignerPayloadJSON).genesisHash !== undefined; -} - export default class Extension { readonly #cachedUnlocks: CachedUnlocks; @@ -368,9 +365,10 @@ export default class Extension { // construct a new registry (avoiding pollution), between requests let registry: Registry; - const { payload } = request; - if (isJsonPayload(payload)) { + if (isExtrinsicRequest(request)) { + const payload = request.payload; + // Get the metadata for the genesisHash const metadata = this.#state.knownMetadata.find(({ genesisHash }) => genesisHash === payload.genesisHash); diff --git a/packages/extension-base/src/background/handlers/Tabs.ts b/packages/extension-base/src/background/handlers/Tabs.ts index 79096244..bbd3b82c 100644 --- a/packages/extension-base/src/background/handlers/Tabs.ts +++ b/packages/extension-base/src/background/handlers/Tabs.ts @@ -141,6 +141,10 @@ export default class Tabs { } private extrinsicSign (url: string, request: SignerPayloadJSON): Promise { + // matches the predicate the UI used to key off, so payloads that were + // never ambiguous (absent, null, empty) keep working + assert(!(request as unknown as SignerPayloadRaw).data, 'Unexpected raw data in a signPayload payload'); + const address = request.address; const pair = this.getSigningPair(address); diff --git a/packages/extension-base/src/background/types.ts b/packages/extension-base/src/background/types.ts index b707ecf6..a5a0f57c 100644 --- a/packages/extension-base/src/background/types.ts +++ b/packages/extension-base/src/background/types.ts @@ -398,12 +398,26 @@ export type SubscriptionMessageTypes = NoUndefinedValues<{ export type MessageTypesWithSubscriptions = keyof SubscriptionMessageTypes; export type MessageTypesWithNoSubscriptions = Exclude -export interface RequestSign { - readonly payload: SignerPayloadJSON | SignerPayloadRaw; - +interface RequestSignBase { sign (registry: Registry, pair: KeyringPair): { signature: HexString }; } +// `channel` is set by the background from the message type the request arrived +// on, never derived from the payload - the payload comes from the dapp and +// cannot be trusted to describe what will actually be signed. Discriminating on +// it also correlates the payload type, so consumers narrow instead of casting. +export interface RequestSignBytes extends RequestSignBase { + readonly channel: 'bytes'; + readonly payload: SignerPayloadRaw; +} + +export interface RequestSignExtrinsic extends RequestSignBase { + readonly channel: 'extrinsic'; + readonly payload: SignerPayloadJSON; +} + +export type RequestSign = RequestSignBytes | RequestSignExtrinsic; + export interface RequestJsonRestore { file: KeyringPair$Json; password: string; diff --git a/packages/extension-base/src/utils/index.ts b/packages/extension-base/src/utils/index.ts index 1bc96ad3..b024b852 100644 --- a/packages/extension-base/src/utils/index.ts +++ b/packages/extension-base/src/utils/index.ts @@ -2,3 +2,4 @@ // SPDX-License-Identifier: Apache-2.0 export { canDerive } from './canDerive.js'; +export { isExtrinsicRequest } from './isExtrinsicRequest.js'; diff --git a/packages/extension-base/src/utils/isExtrinsicRequest.ts b/packages/extension-base/src/utils/isExtrinsicRequest.ts new file mode 100644 index 00000000..2c87c954 --- /dev/null +++ b/packages/extension-base/src/utils/isExtrinsicRequest.ts @@ -0,0 +1,8 @@ +// Copyright 2019-2026 @polkadot/extension-base authors & contributors +// SPDX-License-Identifier: Apache-2.0 + +import type { RequestSign, RequestSignExtrinsic } from '../background/types.js'; + +export function isExtrinsicRequest (request: RequestSign): request is RequestSignExtrinsic { + return request.channel === 'extrinsic'; +} diff --git a/packages/extension-ui/src/Popup/Signing/Request/index.tsx b/packages/extension-ui/src/Popup/Signing/Request/index.tsx index df91772e..7ba56c3c 100644 --- a/packages/extension-ui/src/Popup/Signing/Request/index.tsx +++ b/packages/extension-ui/src/Popup/Signing/Request/index.tsx @@ -3,11 +3,11 @@ import type { AccountJson, RequestSign } from '@polkadot/extension-base/background/types'; import type { ExtrinsicPayload } from '@polkadot/types/interfaces'; -import type { SignerPayloadJSON, SignerPayloadRaw } from '@polkadot/types/types'; import type { HexString } from '@polkadot/util/types'; import React, { useCallback, useContext, useEffect, useState } from 'react'; +import { isExtrinsicRequest } from '@polkadot/extension-base/utils'; import { TypeRegistry } from '@polkadot/types'; import { ActionContext, Address, VerticalSpace, Warning } from '../../../components/index.js'; @@ -39,18 +39,16 @@ export const CMD_SIGN_MESSAGE = 3; // keep it global, we can and will re-use this across requests const registry = new TypeRegistry(); -function isRawPayload (payload: SignerPayloadJSON | SignerPayloadRaw): payload is SignerPayloadRaw { - return !!(payload as SignerPayloadRaw).data; -} - export default function Request ({ account: { accountIndex, addressOffset, genesisHash: accountGenesisHash, isExternal, isHardware, type }, buttonText, isFirst, request, signId, url }: Props): React.ReactElement | null { const onAction = useContext(ActionContext); const [{ hexBytes, payload }, setData] = useState({ hexBytes: null, payload: null }); const [error, setError] = useState(null); const { t } = useTranslation(); + // Raw vs extrinsic follows the channel the request arrived on, never the + // payload fields - those are dapp-supplied and can describe either shape. // Use payload genesis for transaction-signing flow. Account genesis can be null // for allow-any accounts and should not drive payload decoding/signing setup. - const payloadGenesisHash = !isRawPayload(request.payload) + const payloadGenesisHash = isExtrinsicRequest(request) ? request.payload.genesisHash : null; const chain = useMetadata(payloadGenesisHash); @@ -58,25 +56,25 @@ export default function Request ({ account: { accountIndex, addressOffset, genes useEffect((): void => { // When the chain and request are ready, configure the chain's registry. // This will be picked up by LedgerSign. - if (chain && !isRawPayload(request.payload)) { + if (chain && isExtrinsicRequest(request)) { chain.registry.setSignedExtensions(request.payload.signedExtensions, chain.definition.userExtensions); } }, [chain, request]); useEffect((): void => { - const payload = request.payload; + if (isExtrinsicRequest(request)) { + const json = request.payload; - if (isRawPayload(payload)) { - setData({ - hexBytes: payload.data, - payload: null - }); - } else { - registry.setSignedExtensions(payload.signedExtensions); + registry.setSignedExtensions(json.signedExtensions); setData({ hexBytes: null, - payload: registry.createType('ExtrinsicPayload', payload, { version: payload.version }) + payload: registry.createType('ExtrinsicPayload', json, { version: json.version }) + }); + } else { + setData({ + hexBytes: request.payload.data, + payload: null }); } }, [request]); @@ -93,8 +91,15 @@ export default function Request ({ account: { accountIndex, addressOffset, genes [onAction, signId] ); - if (payload !== null) { - const json = request.payload as SignerPayloadJSON; + // Branch on the request itself rather than on the decoded state, so a render + // that lands before the effect has caught up shows nothing instead of feeding + // the previous request's view a payload of the other shape. + if (isExtrinsicRequest(request)) { + const json = request.payload; + + if (payload === null) { + return null; + } return ( <> @@ -147,8 +152,12 @@ export default function Request ({ account: { accountIndex, addressOffset, genes /> ); - } else if (hexBytes !== null) { - const { address, data } = request.payload as SignerPayloadRaw; + } else { + const { address, data } = request.payload; + + if (hexBytes === null) { + return null; + } return ( <> @@ -197,6 +206,4 @@ export default function Request ({ account: { accountIndex, addressOffset, genes ); } - - return null; } diff --git a/packages/extension-ui/src/Popup/Signing/Signing.spec.tsx b/packages/extension-ui/src/Popup/Signing/Signing.spec.tsx index b3d220ff..a69e5d30 100644 --- a/packages/extension-ui/src/Popup/Signing/Signing.spec.tsx +++ b/packages/extension-ui/src/Popup/Signing/Signing.spec.tsx @@ -95,6 +95,7 @@ describe('Signing requests', () => { }, id: '1607347015530.2', request: { + channel: 'extrinsic', payload: { address: '5D4bqjQRPgdMBK8bNvhX4tSuCtSGZS7rZjD5XH5SoKcFeKn5', blockHash: '0x661f57d206d4fecda0408943427d4d25436518acbff543735e7569da9db6bdd7', @@ -132,6 +133,7 @@ describe('Signing requests', () => { }, id: '1607356155395.3', request: { + channel: 'extrinsic', payload: { address: '5Ggap6soAPaP5UeNaiJsgqQwdVhhNnm6ez7Ba1w9jJ62LM2Q', blockHash: '0xcf69b7935b785f90b22d2b36f2227132ef9c5dd33db1dbac9ecdafac05bf9476', @@ -221,6 +223,7 @@ describe('Signing requests', () => { }, id: '1607357806151.5', request: { + channel: 'extrinsic', payload: { address: '5Cf1CGZas62RWwce3d2EPqUvSoi1txaXKd9M5w9bEFSsQtRe', blockHash: '0xd2f2dfb56c16af1d0faf5b454153d3199aeb6647537f4161c26a34541c591ec8', diff --git a/packages/extension-ui/src/Popup/Signing/index.tsx b/packages/extension-ui/src/Popup/Signing/index.tsx index bc01d7cb..8c2a6157 100644 --- a/packages/extension-ui/src/Popup/Signing/index.tsx +++ b/packages/extension-ui/src/Popup/Signing/index.tsx @@ -1,10 +1,10 @@ // Copyright 2019-2026 @polkadot/extension-ui authors & contributors // SPDX-License-Identifier: Apache-2.0 -import type { SignerPayloadJSON } from '@polkadot/types/types'; - import React, { useCallback, useContext, useEffect, useState } from 'react'; +import { isExtrinsicRequest } from '@polkadot/extension-base/utils'; + import { Loading, SigningReqContext } from '../../components/index.js'; import { useTranslation } from '../../hooks/index.js'; import { Header } from '../../partials/index.js'; @@ -42,7 +42,7 @@ export default function Signing (): React.ReactElement { : requests[requests.length - 1] : requests[0] : null; - const isTransaction = !!((request?.request?.payload as SignerPayloadJSON)?.blockNumber); + const isTransaction = !!request && isExtrinsicRequest(request.request); return request ? (