From 451257ec7fb17c3ee648228941dd037d55a1a1e4 Mon Sep 17 00:00:00 2001 From: bragaz Date: Tue, 25 Aug 2026 01:15:25 +0200 Subject: [PATCH 1/4] fix(ledger): derive the signer from the selected account index `getAddress` built its account with the selected `accountIndex` while the signer was built with `createFromLedger(transport)`, which defaults to index 0. On a multi-account Ledger the address shown to the user was not the key that signed. Hoist the index and use it for both. Co-Authored-By: Claude Opus 5 --- hooks/wallet/useKaspaLedgerSigner.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hooks/wallet/useKaspaLedgerSigner.ts b/hooks/wallet/useKaspaLedgerSigner.ts index 6a48e3f9..5671be42 100644 --- a/hooks/wallet/useKaspaLedgerSigner.ts +++ b/hooks/wallet/useKaspaLedgerSigner.ts @@ -20,8 +20,11 @@ export default function useKaspaLedgerSigner() { ? new LegacyAccountFactory() : new AccountFactory(); + // the signer and getAddress must derive from the same index, otherwise the + // address shown to the user is not the key that signs + const accountIndex = account?.index ?? 0; + const getAddress = async () => { - const accountIndex = account?.index ?? 0; const publicKey = await factory .createFromLedger(transport, accountIndex) .getPublicKey(); @@ -29,7 +32,7 @@ export default function useKaspaLedgerSigner() { return address; }; - const signer = factory.createFromLedger(transport); + const signer = factory.createFromLedger(transport, accountIndex); return { getAddress, From e468a9531d68cc6abfb5b768ac15ff33e30f247e Mon Sep 17 00:00:00 2001 From: bragaz Date: Tue, 25 Aug 2026 01:15:36 +0200 Subject: [PATCH 2/4] fix(sign): refuse sighash types that commit no outputs, warn on partial ones A connected dApp could request `None` or `NoneAnyOneCanPay` for a script option. Those sighash types commit no outputs at all, so the amount and destination shown on the confirm screen are not bound by the signature and can be rewritten after the user approves. `normalizeScriptOptions` now refuses them behind a single `ALLOW_UNSAFE_OUTPUT_SIGHASH` constant. `Single` and `SingleAnyOneCanPay` commit only the same-index output, which is what KaspaCom PSKT listings rely on, so they stay signable; `SignTx` now passes `SignConfirm`'s existing `warning` prop when any option uses one, telling the user the remaining outputs can still change. The masking each sighash type applies is proven against the WASM signer rather than assumed: the spec rebuilds the signing preimage per type and checks the WASM signature verifies against it, and that it stops verifying exactly when a committed output changes. Co-Authored-By: Claude Opus 5 --- .../browser-api/kaspa/sign-tx/SignTx.tsx | 9 + lib/wallet/sign-script.ts | 42 ++- tests/signtx-unit.spec.ts | 322 ++++++++++++++++-- 3 files changed, 343 insertions(+), 30 deletions(-) diff --git a/components/screens/browser-api/kaspa/sign-tx/SignTx.tsx b/components/screens/browser-api/kaspa/sign-tx/SignTx.tsx index 87db36c6..f055ba49 100644 --- a/components/screens/browser-api/kaspa/sign-tx/SignTx.tsx +++ b/components/screens/browser-api/kaspa/sign-tx/SignTx.tsx @@ -6,6 +6,10 @@ import SignConfirm from "@/components/screens/browser-api/kaspa/sign/SignConfirm import { ApiUtils } from "@/api/background/utils"; import useAnalytics from "@/hooks/useAnalytics"; import { deserializeTransaction } from "@/lib/kaspa-compat"; +import { hasPartialOutputCommitment } from "@/lib/wallet/sign-script.ts"; + +const PARTIAL_OUTPUT_WARNING = + "This request signs with a sighash type that commits to only part of the transaction outputs. The outputs it does not cover are not protected by your signature and can still be changed after you approve."; type SignTxProps = { wallet: IWallet; @@ -65,6 +69,11 @@ export default function SignTx({ confirm={handleConfirm} cancel={handleCancel} payload={payload} + warning={ + hasPartialOutputCommitment(payload.scripts) + ? PARTIAL_OUTPUT_WARNING + : undefined + } /> ); diff --git a/lib/wallet/sign-script.ts b/lib/wallet/sign-script.ts index 2ad19e59..d4d0720b 100644 --- a/lib/wallet/sign-script.ts +++ b/lib/wallet/sign-script.ts @@ -13,6 +13,36 @@ export type RawScriptOption = Partial & { script?: string }; const HEX_RE = /^(?:[0-9a-fA-F]{2})*$/; +// Sighash types split on one axis: how much of the output set the signature +// commits to. +// All / AllAnyOneCanPay -> every output committed (safe) +// Single / SingleAnyOneCanPay -> only the same-index output (partial) +// None / NoneAnyOneCanPay -> no outputs committed at all (unsafe) +// With None*, the amount and destination the confirm screen showed are not +// bound by the signature and can be rewritten after approval, so they are +// refused outright. +// TODO: allow per-request opt-in from an advanced setting instead of a +// hardcoded constant, once that setting exists. +const ALLOW_UNSAFE_OUTPUT_SIGHASH = false; + +const UNSAFE_OUTPUT_SIGN_TYPES = ["None", "NoneAnyOneCanPay"] as const; +const PARTIAL_OUTPUT_SIGN_TYPES = ["Single", "SingleAnyOneCanPay"] as const; + +/** + * True when any option signs with a sighash type that commits only part of the + * outputs, so the rest can still change after the user approves. Non-throwing: + * safe to call from render paths that also display invalid requests. + */ +export function hasPartialOutputCommitment( + scripts?: (RawScriptOption | null | undefined)[], +): boolean { + return (scripts ?? []).some((option) => + (PARTIAL_OUTPUT_SIGN_TYPES as readonly string[]).includes( + option?.signType ?? "All", + ), + ); +} + export function normalizeScriptOptions( scripts?: (RawScriptOption | null | undefined)[], ): ScriptOption[] { @@ -38,7 +68,17 @@ export function normalizeScriptOptions( throw new Error(`signTx: invalid scriptHex for input ${inputIndex}`); } - return { inputIndex, scriptHex, signType: option.signType ?? "All" }; + const signType = option.signType ?? "All"; + if ( + !ALLOW_UNSAFE_OUTPUT_SIGHASH && + (UNSAFE_OUTPUT_SIGN_TYPES as readonly string[]).includes(signType) + ) { + throw new Error( + `signTx: signType "${signType}" commits no outputs, so every output could be rewritten after signing; refusing to sign input ${inputIndex}`, + ); + } + + return { inputIndex, scriptHex, signType }; }); } diff --git a/tests/signtx-unit.spec.ts b/tests/signtx-unit.spec.ts index 6aff0b1e..5a7c0755 100644 --- a/tests/signtx-unit.spec.ts +++ b/tests/signtx-unit.spec.ts @@ -13,11 +13,14 @@ import init, { } from "@/wasm/core/kaspa"; import { deserializeTransaction } from "@/lib/kaspa-compat"; import { + hasPartialOutputCommitment, normalizeScriptOptions, pushDataHex, signTxWithScriptOptions, type RawScriptOption, } from "@/lib/wallet/sign-script"; +import { SIGN_TYPE, toSignType } from "@/lib/kaspa"; +import type { SignType } from "@/lib/wallet/wallet-interface"; // Throwaway key — unit tests only, never funded. const TEST_KEY = @@ -299,33 +302,63 @@ test.describe("sighash payload coverage (KIP-12)", () => { return out; }; - // SigHashAll preimage per rusty-kaspa's TransactionSigningHash - function sighashAll(tx: any, inputIndex: number) { - const prevouts = keyedHash( - cat( - ...tx.inputs.map((i: any) => - cat(hex2b(i.transactionId), new Uint8Array(4)), - ), - ), - ); - const sequences = keyedHash( - cat(...tx.inputs.map((i: any) => u64le(i.sequence))), - ); - const sigOpCounts = keyedHash( - cat(...tx.inputs.map((i: any) => Uint8Array.of(i.sigOpCount))), + // Consensus wire byte per sighash type (rusty-kaspa SigHashType bits); the + // SIGN_TYPE enum values are just indices into this set. Note AllAnyOneCanPay + // is the bare ANY_ONE_CAN_PAY bit, not ALL|ANY_ONE_CAN_PAY — the type-name + // arithmetic would give 0x81. Every byte here is asserted against the byte + // the WASM signer actually emits, below. + const HASH_TYPE_BYTE: Record = { + All: 0b0000_0001, + None: 0b0000_0010, + Single: 0b0000_0100, + AllAnyOneCanPay: 0b1000_0000, + NoneAnyOneCanPay: 0b1000_0010, + SingleAnyOneCanPay: 0b1000_0100, + }; + + const ZERO32 = new Uint8Array(32); + const hashOutput = (o: any) => + cat( + u64le(o.value), + u16le(parseInt(o.scriptPublicKey.slice(0, 4), 16)), + u64le((o.scriptPublicKey.length - 4) / 2), + hex2b(o.scriptPublicKey.slice(4)), ); - const outputs = keyedHash( - cat( - ...tx.outputs.map((o: any) => + + // Signing preimage per rusty-kaspa's TransactionSigningHash, including the + // field masking each sighash type applies. This is what makes the + // output-commitment claims below verifiable rather than assumed. + function sighashFor(tx: any, inputIndex: number, type: SignType = "All") { + const anyoneCanPay = type.endsWith("AnyOneCanPay"); + const isNone = type.startsWith("None"); + const isSingle = type.startsWith("Single"); + + const prevouts = anyoneCanPay + ? ZERO32 + : keyedHash( cat( - u64le(o.value), - u16le(parseInt(o.scriptPublicKey.slice(0, 4), 16)), - u64le((o.scriptPublicKey.length - 4) / 2), - hex2b(o.scriptPublicKey.slice(4)), + ...tx.inputs.map((i: any) => + cat(hex2b(i.transactionId), new Uint8Array(4)), + ), ), - ), - ), - ); + ); + const sequences = + anyoneCanPay || isNone || isSingle + ? ZERO32 + : keyedHash(cat(...tx.inputs.map((i: any) => u64le(i.sequence)))); + const sigOpCounts = anyoneCanPay + ? ZERO32 + : keyedHash( + cat(...tx.inputs.map((i: any) => Uint8Array.of(i.sigOpCount))), + ); + // None commits no outputs at all; Single commits only the same-index one. + const outputs = isNone + ? ZERO32 + : isSingle + ? inputIndex >= tx.outputs.length + ? ZERO32 + : keyedHash(hashOutput(tx.outputs[inputIndex])) + : keyedHash(cat(...tx.outputs.map(hashOutput))); // KIP-12: payload committed as keyed hash of length-prefixed payload bytes const payloadHash = tx.payload && tx.payload.length > 0 @@ -353,11 +386,99 @@ test.describe("sighash payload coverage (KIP-12)", () => { hex2b(tx.subnetworkId), u64le(tx.gas), payloadHash, - Uint8Array.of(1), // SigHashAll + Uint8Array.of(HASH_TYPE_BYTE[type]), ), ); } + // The claim the None* block and the Single* warning rest on: how much of the + // output set each sighash type actually commits to. Proven against the WASM + // signer rather than assumed — a signature only verifies if our preimage, + // including its output masking, matches what the WASM signed. + test("output commitment per sighash type matches the masking we gate on", () => { + const priv = new PrivateKey(TEST_KEY); + const pubX = priv.toPublicKey().toXOnlyPublicKey().toString(); + const spk = "0000" + "20" + pubX + "ac"; + const mkTx = (out0: string, out1: string) => ({ + id: "00".repeat(32), + version: 0, + inputs: [ + { + transactionId: "11".repeat(32), + index: 0, + sequence: "0", + sigOpCount: 1, + computeBudget: 0, + signatureScript: "", + utxo: { + address: null, + amount: "100000000", + scriptPublicKey: spk, + blockDaaScore: "1000", + isCoinbase: false, + }, + }, + ], + outputs: [ + { value: out0, scriptPublicKey: spk }, + { value: out1, scriptPublicKey: spk }, + ], + lockTime: "0", + subnetworkId: "00".repeat(20), + gas: "0", + payload: "", + }); + + const base = mkTx("40000000", "40000000"); + const sameIndexChanged = mkTx("10000000", "40000000"); // output 0 rewritten + const otherIndexChanged = mkTx("40000000", "10000000"); // output 1 rewritten + + // outputs the signature is expected to bind, per type + const commits: Record< + SignType, + { sameIndex: boolean; otherIndex: boolean } + > = { + All: { sameIndex: true, otherIndex: true }, + AllAnyOneCanPay: { sameIndex: true, otherIndex: true }, + Single: { sameIndex: true, otherIndex: false }, + SingleAnyOneCanPay: { sameIndex: true, otherIndex: false }, + None: { sameIndex: false, otherIndex: false }, + NoneAnyOneCanPay: { sameIndex: false, otherIndex: false }, + }; + + for (const type of Object.keys(SIGN_TYPE) as SignType[]) { + const wasmTx = Transaction.deserializeFromSafeJSON(JSON.stringify(base)); + const sigScript = createInputSignature(wasmTx, 0, priv, toSignType(type)); + const sig64 = hex2b(sigScript.slice(2, 2 + 128)); + + // the trailing byte of the signature is the consensus hash-type byte + expect(sigScript.slice(2 + 128)).toBe( + HASH_TYPE_BYTE[type].toString(16).padStart(2, "0"), + ); + + // our masked preimage reproduces exactly what the WASM signed + expect( + schnorr.verify(sig64, sighashFor(base, 0, type), hex2b(pubX)), + ).toBe(true); + + // a signature that commits an output cannot survive that output changing + expect( + schnorr.verify( + sig64, + sighashFor(sameIndexChanged, 0, type), + hex2b(pubX), + ), + ).toBe(!commits[type].sameIndex); + expect( + schnorr.verify( + sig64, + sighashFor(otherIndexChanged, 0, type), + hex2b(pubX), + ), + ).toBe(!commits[type].otherIndex); + } + }); + test("a payload-bearing tx signs a different sighash than the same tx with empty payload", () => { const priv = new PrivateKey(TEST_KEY); const pubX = priv.toPublicKey().toXOnlyPublicKey().toString(); @@ -392,8 +513,8 @@ test.describe("sighash payload coverage (KIP-12)", () => { const withPayload = mkTx("beef1234"); const withoutPayload = mkTx(""); - const hashWith = sighashAll(withPayload, 0); - const hashWithout = sighashAll(withoutPayload, 0); + const hashWith = sighashFor(withPayload, 0); + const hashWithout = sighashFor(withoutPayload, 0); expect(Buffer.from(hashWith).toString("hex")).not.toBe( Buffer.from(hashWithout).toString("hex"), ); @@ -405,13 +526,156 @@ test.describe("sighash payload coverage (KIP-12)", () => { const sig64 = hex2b(sigScript.slice(2, 2 + 128)); // WASM signature verifies against OUR sighash for the same payload... - expect(schnorr.verify(sig64, sighashAll(tx, 0), hex2b(pubX))).toBe(true); + expect(schnorr.verify(sig64, sighashFor(tx, 0), hex2b(pubX))).toBe(true); // ...and NOT against the sighash with the payload flipped — so the // signed message really commits to tx.payload. const flipped = tx === withPayload ? withoutPayload : withPayload; - expect(schnorr.verify(sig64, sighashAll(flipped, 0), hex2b(pubX))).toBe( + expect(schnorr.verify(sig64, sighashFor(flipped, 0), hex2b(pubX))).toBe( + false, + ); + } + }); +}); + +test.describe("sighash safety policy (U1)", () => { + const fixtureTx = () => deserializeTransaction(loadFixture().txJson); + + test("refuses sighash types that commit no outputs", async () => { + for (const signType of ["None", "NoneAnyOneCanPay"] as const) { + expect(() => + normalizeScriptOptions([ + { inputIndex: 1, scriptHex: "aabb", signType }, + ]), + ).toThrow( + `signTx: signType "${signType}" commits no outputs, so every output could be rewritten after signing; refusing to sign input 1`, + ); + + await expect( + signTxWithScriptOptions( + fixtureTx(), + [{ inputIndex: 1, scriptHex: "aabb", signType }], + TEST_KEY, + ), + ).rejects.toThrow(`signTx: signType "${signType}" commits no outputs`); + } + }); + + test("a blocked option fails the call before any input is mutated", async () => { + const { txJson, scripts } = loadFixture(); + const tx = deserializeTransaction(txJson); + await expect( + signTxWithScriptOptions( + tx, + [ + { inputIndex: 1, scriptHex: scripts[0].scriptHex }, + { inputIndex: 2, scriptHex: "aabb", signType: "None" }, + ], + TEST_KEY, + ), + ).rejects.toThrow('signTx: signType "None" commits no outputs'); + expect(JSON.parse(tx.serializeToSafeJSON()).inputs[1].signatureScript).toBe( + "", + ); + }); + + test("partial-commitment sighash types stay signable (KaspaCom PSKT listings)", async () => { + const { txJson, scripts } = loadFixture(); + for (const signType of ["Single", "SingleAnyOneCanPay"] as const) { + expect( + normalizeScriptOptions([ + { inputIndex: 1, scriptHex: "aabb", signType }, + ]), + ).toEqual([{ inputIndex: 1, scriptHex: "aabb", signType }]); + + const tx = deserializeTransaction(txJson); + const signed = await signTxWithScriptOptions( + tx, + [{ inputIndex: 1, scriptHex: scripts[0].scriptHex, signType }], + TEST_KEY, + ); + expect( + JSON.parse(signed.serializeToSafeJSON()).inputs[1].signatureScript, + ).not.toBe(""); + } + }); + + test("full-commitment sighash types stay signable and default to All", async () => { + const { txJson, scripts } = loadFixture(); + for (const signType of ["All", "AllAnyOneCanPay"] as const) { + expect( + normalizeScriptOptions([ + { inputIndex: 1, scriptHex: "aabb", signType }, + ]), + ).toEqual([{ inputIndex: 1, scriptHex: "aabb", signType }]); + } + + // absent signType resolves to All + expect( + normalizeScriptOptions([{ inputIndex: 1, scriptHex: "aabb" }]), + ).toEqual([{ inputIndex: 1, scriptHex: "aabb", signType: "All" }]); + + const signed = await signTxWithScriptOptions( + deserializeTransaction(txJson), + scripts, + TEST_KEY, + ); + expect( + JSON.parse(signed.serializeToSafeJSON()).inputs[1].signatureScript, + ).not.toBe(""); + }); + + test("warns only for sighash types that commit part of the outputs", () => { + for (const signType of ["Single", "SingleAnyOneCanPay"] as const) { + expect(hasPartialOutputCommitment([{ inputIndex: 0, signType }])).toBe( + true, + ); + // one flagged option among safe ones is enough to warn + expect( + hasPartialOutputCommitment([ + { inputIndex: 0, signType: "All" }, + { inputIndex: 1, signType }, + ]), + ).toBe(true); + } + + for (const signType of ["All", "AllAnyOneCanPay"] as const) { + expect(hasPartialOutputCommitment([{ inputIndex: 0, signType }])).toBe( false, ); } + expect(hasPartialOutputCommitment([{ inputIndex: 0 }])).toBe(false); + expect(hasPartialOutputCommitment([null, undefined])).toBe(false); + expect(hasPartialOutputCommitment([])).toBe(false); + expect(hasPartialOutputCommitment(undefined)).toBe(false); + }); + + test("toSignType keeps its prototype-pollution guard", () => { + for (const protoKey of ["__proto__", "toString", "constructor"]) { + expect(() => toSignType(protoKey as any)).toThrow( + `signTx: unsupported signType "${protoKey}"`, + ); + } + for (const signType of Object.keys(SIGN_TYPE) as SignType[]) { + expect(toSignType(signType)).toBe(SIGN_TYPE[signType]); + } + }); +}); + +// F7: the address the user is shown and the key that signs must come from the +// same derivation index. Driving a real Ledger needs a Transport, so this is a +// source-level assertion on the hook that builds both. +test.describe("ledger signer index parity (F7)", () => { + test("every createFromLedger call in useKaspaLedgerSigner passes an index", () => { + const source = fs.readFileSync( + path.join(TESTS_DIR, "../hooks/wallet/useKaspaLedgerSigner.ts"), + "utf8", + ); + const calls = [...source.matchAll(/createFromLedger\(([^)]*)\)/g)].map( + (m) => m[1].split(",").map((a) => a.trim()), + ); + expect(calls.length).toBeGreaterThan(1); + for (const args of calls) { + expect(args).toEqual(["transport", "accountIndex"]); + } }); }); From dd18186ef4d8de5747690fa5e663546074b2af95 Mon Sep 17 00:00:00 2001 From: bragaz Date: Tue, 25 Aug 2026 12:06:53 +0200 Subject: [PATCH 3/4] fix(sign): show the partial-commitment warning on SignAndBroadcast too SignAndBroadcast passes payload.scripts to wallet.signTx exactly like SignTx, but only carried the fee-adjustment notice. Reuse the same PARTIAL_OUTPUT_WARNING copy (now exported from SignTx) and compose it with the fee notice as separate paragraphs (joined with a blank line; SignConfirm renders the warning box with whitespace-pre-line), so both stay visible when both apply. Co-Authored-By: Claude Fable 5 --- .../kaspa/sign-and-broadcast/SignAndBroadcast.tsx | 15 ++++++++++++--- .../screens/browser-api/kaspa/sign-tx/SignTx.tsx | 2 +- .../browser-api/kaspa/sign/SignConfirm.tsx | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/components/screens/browser-api/kaspa/sign-and-broadcast/SignAndBroadcast.tsx b/components/screens/browser-api/kaspa/sign-and-broadcast/SignAndBroadcast.tsx index e79d2e94..17c5f917 100644 --- a/components/screens/browser-api/kaspa/sign-and-broadcast/SignAndBroadcast.tsx +++ b/components/screens/browser-api/kaspa/sign-and-broadcast/SignAndBroadcast.tsx @@ -13,6 +13,8 @@ import { TransactionOutput, } from "@/wasm/core/kaspa"; import { calcRevealInputMass } from "@/lib/kaspaFee"; +import { hasPartialOutputCommitment } from "@/lib/wallet/sign-script.ts"; +import { PARTIAL_OUTPUT_WARNING } from "@/components/screens/browser-api/kaspa/sign-tx/SignTx"; type SignAndBroadcastProps = { wallet: IWallet; @@ -156,9 +158,16 @@ export default function SignAndBroadcast({ cancel={handleCancel} confirm={handleConfirm} warning={ - feeAdjusted - ? "Transaction fee was insufficient. Network fee has been automatically adjusted." - : undefined + [ + hasPartialOutputCommitment(payload.scripts) + ? PARTIAL_OUTPUT_WARNING + : undefined, + feeAdjusted + ? "Transaction fee was insufficient. Network fee has been automatically adjusted." + : undefined, + ] + .filter(Boolean) + .join("\n\n") || undefined } /> ); diff --git a/components/screens/browser-api/kaspa/sign-tx/SignTx.tsx b/components/screens/browser-api/kaspa/sign-tx/SignTx.tsx index f055ba49..e70a565a 100644 --- a/components/screens/browser-api/kaspa/sign-tx/SignTx.tsx +++ b/components/screens/browser-api/kaspa/sign-tx/SignTx.tsx @@ -8,7 +8,7 @@ import useAnalytics from "@/hooks/useAnalytics"; import { deserializeTransaction } from "@/lib/kaspa-compat"; import { hasPartialOutputCommitment } from "@/lib/wallet/sign-script.ts"; -const PARTIAL_OUTPUT_WARNING = +export const PARTIAL_OUTPUT_WARNING = "This request signs with a sighash type that commits to only part of the transaction outputs. The outputs it does not cover are not protected by your signature and can still be changed after you approve."; type SignTxProps = { diff --git a/components/screens/browser-api/kaspa/sign/SignConfirm.tsx b/components/screens/browser-api/kaspa/sign/SignConfirm.tsx index 1082922b..532aee5d 100644 --- a/components/screens/browser-api/kaspa/sign/SignConfirm.tsx +++ b/components/screens/browser-api/kaspa/sign/SignConfirm.tsx @@ -207,7 +207,7 @@ export default function SignConfirm({ {!networkMismatched && ( <> {warning && ( -
+
{warning}
)} From 4c50bba57b287144a9c999a89ce815209c7fdee3 Mon Sep 17 00:00:00 2001 From: bragaz Date: Tue, 25 Aug 2026 12:06:53 +0200 Subject: [PATCH 4/4] fix(sign): refuse None* inside signTxInputWithScriptOption; test evasion vectors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the None* refusal structural rather than conventional: extract assertSafeOutputSighash and call it both from normalizeScriptOptions (early, before any mutation — kept for atomicity) and from the exported signTxInputWithScriptOption itself, so a future caller that skips normalization cannot silently reintroduce unsafe signing. Add a test calling signTxInputWithScriptOption directly with None*. Extend the rejected-signType loop with the uncovered evasion vectors: lowercase "none", trailing-whitespace "None ", and the raw numeric enum value 2 (SighashType.None). Each must throw with no signature produced; the loop now also asserts the input stays unmutated. Co-Authored-By: Claude Fable 5 --- lib/wallet/sign-script.ts | 26 +++++++++++++++-------- tests/signtx-unit.spec.ts | 43 ++++++++++++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/lib/wallet/sign-script.ts b/lib/wallet/sign-script.ts index d4d0720b..f75ee1cb 100644 --- a/lib/wallet/sign-script.ts +++ b/lib/wallet/sign-script.ts @@ -28,6 +28,21 @@ const ALLOW_UNSAFE_OUTPUT_SIGHASH = false; const UNSAFE_OUTPUT_SIGN_TYPES = ["None", "NoneAnyOneCanPay"] as const; const PARTIAL_OUTPUT_SIGN_TYPES = ["Single", "SingleAnyOneCanPay"] as const; +// Chokepoint for the None* refusal: called from normalizeScriptOptions (fails +// the whole request early, before any mutation) AND from +// signTxInputWithScriptOption itself, so a future caller that skips +// normalization cannot silently reintroduce unsafe signing. +function assertSafeOutputSighash(signType: string, inputIndex: number): void { + if ( + !ALLOW_UNSAFE_OUTPUT_SIGHASH && + (UNSAFE_OUTPUT_SIGN_TYPES as readonly string[]).includes(signType) + ) { + throw new Error( + `signTx: signType "${signType}" commits no outputs, so every output could be rewritten after signing; refusing to sign input ${inputIndex}`, + ); + } +} + /** * True when any option signs with a sighash type that commits only part of the * outputs, so the rest can still change after the user approves. Non-throwing: @@ -69,14 +84,7 @@ export function normalizeScriptOptions( } const signType = option.signType ?? "All"; - if ( - !ALLOW_UNSAFE_OUTPUT_SIGHASH && - (UNSAFE_OUTPUT_SIGN_TYPES as readonly string[]).includes(signType) - ) { - throw new Error( - `signTx: signType "${signType}" commits no outputs, so every output could be rewritten after signing; refusing to sign input ${inputIndex}`, - ); - } + assertSafeOutputSighash(signType, inputIndex); return { inputIndex, scriptHex, signType }; }); @@ -111,6 +119,8 @@ export function signTxInputWithScriptOption( option: ScriptOption, privateKeyString: string, ) { + assertSafeOutputSighash(option.signType ?? "All", option.inputIndex); + // each tx.inputs access crosses the WASM boundary; read it once const inputs = tx.inputs; if (option.inputIndex >= inputs.length) { diff --git a/tests/signtx-unit.spec.ts b/tests/signtx-unit.spec.ts index 5a7c0755..86f0b44d 100644 --- a/tests/signtx-unit.spec.ts +++ b/tests/signtx-unit.spec.ts @@ -16,6 +16,7 @@ import { hasPartialOutputCommitment, normalizeScriptOptions, pushDataHex, + signTxInputWithScriptOption, signTxWithScriptOptions, type RawScriptOption, } from "@/lib/wallet/sign-script"; @@ -198,15 +199,29 @@ test.describe("signTx covenant repro (kaspa.com fixture)", () => { ), ).rejects.toThrow('signTx: unsupported signType "Bogus"'); - // inherited object keys must not resolve to sighash types - for (const protoKey of ["toString", "constructor", "__proto__"]) { + // inherited object keys must not resolve to sighash types, and neither + // may near-miss evasions of the refused names: lowercase, trailing + // whitespace, or the raw numeric enum value (2 = SighashType.None). + // Each must throw before any signature is produced. + for (const badType of [ + "toString", + "constructor", + "__proto__", + "none", + "None ", + 2, + ]) { + const evasionTx = deserializeTransaction(txJson); await expect( signTxWithScriptOptions( - deserializeTransaction(txJson), - [{ inputIndex: 1, scriptHex: "aa", signType: protoKey as any }], + evasionTx, + [{ inputIndex: 1, scriptHex: "aa", signType: badType as any }], TEST_KEY, ), - ).rejects.toThrow(`signTx: unsupported signType "${protoKey}"`); + ).rejects.toThrow(`signTx: unsupported signType "${badType}"`); + expect( + JSON.parse(evasionTx.serializeToSafeJSON()).inputs[1].signatureScript, + ).toBe(""); } await expect( @@ -560,6 +575,24 @@ test.describe("sighash safety policy (U1)", () => { } }); + test("signTxInputWithScriptOption refuses None* when called directly, bypassing normalization", () => { + for (const signType of ["None", "NoneAnyOneCanPay"] as const) { + const tx = fixtureTx(); + expect(() => + signTxInputWithScriptOption( + tx, + { inputIndex: 1, scriptHex: "aabb", signType }, + TEST_KEY, + ), + ).toThrow( + `signTx: signType "${signType}" commits no outputs, so every output could be rewritten after signing; refusing to sign input 1`, + ); + expect( + JSON.parse(tx.serializeToSafeJSON()).inputs[1].signatureScript, + ).toBe(""); + } + }); + test("a blocked option fails the call before any input is mutated", async () => { const { txJson, scripts } = loadFixture(); const tx = deserializeTransaction(txJson);