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 87db36c6..e70a565a 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"; + +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 = { 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/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}
)} 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, diff --git a/lib/wallet/sign-script.ts b/lib/wallet/sign-script.ts index 2ad19e59..f75ee1cb 100644 --- a/lib/wallet/sign-script.ts +++ b/lib/wallet/sign-script.ts @@ -13,6 +13,51 @@ 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; + +// 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: + * 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 +83,10 @@ export function normalizeScriptOptions( throw new Error(`signTx: invalid scriptHex for input ${inputIndex}`); } - return { inputIndex, scriptHex, signType: option.signType ?? "All" }; + const signType = option.signType ?? "All"; + assertSafeOutputSighash(signType, inputIndex); + + return { inputIndex, scriptHex, signType }; }); } @@ -71,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 6aff0b1e..86f0b44d 100644 --- a/tests/signtx-unit.spec.ts +++ b/tests/signtx-unit.spec.ts @@ -13,11 +13,15 @@ import init, { } from "@/wasm/core/kaspa"; import { deserializeTransaction } from "@/lib/kaspa-compat"; import { + hasPartialOutputCommitment, normalizeScriptOptions, pushDataHex, + signTxInputWithScriptOption, 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 = @@ -195,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( @@ -299,33 +317,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 +401,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 +528,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 +541,174 @@ 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("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); + 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"]); + } }); });