fix: derive raw vs extrinsic signing from the request channel (#1624)
The signing popup decided whether to show the message view or the transaction view by inspecting fields on the payload (`data`, `blockNumber`), while the background picks the signing behaviour from the message channel the request arrived on. Those two are independent and the payload comes from the dapp, so they can disagree. `RequestSign` becomes a discriminated union on `channel`, set by the background from the message type. Correlating the discriminant with the payload type means consumers narrow through `isExtrinsicRequest` rather than casting, so the display and the signing path can no longer drift apart. `Tabs.extrinsicSign` also rejects a payload carrying a truthy `data`, since that field has no meaning on that channel. Also branches the signing render on the request rather than on decoded state. Paging between two queued requests of different shapes could otherwise commit one frame using the previous request's decoded payload. Tests added under `signer channel` in `Extension.spec.ts`.
This commit is contained in:
parent
58093e3a50
commit
e66446edc3
@ -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) {
|
||||
|
||||
@ -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) {
|
||||
|
||||
@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -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);
|
||||
|
||||
|
||||
@ -141,6 +141,10 @@ export default class Tabs {
|
||||
}
|
||||
|
||||
private extrinsicSign (url: string, request: SignerPayloadJSON): Promise<ResponseSigning> {
|
||||
// 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);
|
||||
|
||||
|
||||
@ -398,12 +398,26 @@ export type SubscriptionMessageTypes = NoUndefinedValues<{
|
||||
export type MessageTypesWithSubscriptions = keyof SubscriptionMessageTypes;
|
||||
export type MessageTypesWithNoSubscriptions = Exclude<MessageTypes, keyof SubscriptionMessageTypes>
|
||||
|
||||
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;
|
||||
|
||||
@ -2,3 +2,4 @@
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
|
||||
export { canDerive } from './canDerive.js';
|
||||
export { isExtrinsicRequest } from './isExtrinsicRequest.js';
|
||||
|
||||
8
packages/extension-base/src/utils/isExtrinsicRequest.ts
Normal file
8
packages/extension-base/src/utils/isExtrinsicRequest.ts
Normal file
@ -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';
|
||||
}
|
||||
@ -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<Props> | null {
|
||||
const onAction = useContext(ActionContext);
|
||||
const [{ hexBytes, payload }, setData] = useState<Data>({ hexBytes: null, payload: null });
|
||||
const [error, setError] = useState<string | null>(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;
|
||||
}
|
||||
|
||||
@ -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',
|
||||
|
||||
@ -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
|
||||
? (
|
||||
|
||||
Loading…
Reference in New Issue
Block a user