diff --git a/modules/abstract-utxo/package.json b/modules/abstract-utxo/package.json index b68c31f58d..c79056e0d2 100644 --- a/modules/abstract-utxo/package.json +++ b/modules/abstract-utxo/package.json @@ -67,7 +67,7 @@ "@bitgo/utxo-core": "^1.41.4", "@bitgo/utxo-descriptors": "^2.0.0", "@bitgo/utxo-ord": "^1.34.4", - "@bitgo/wasm-utxo": "^5.5.1", + "@bitgo/wasm-utxo": "^5.6.0", "@types/lodash": "^4.14.121", "@types/superagent": "4.1.15", "bignumber.js": "^9.0.2", diff --git a/modules/abstract-utxo/src/recovery/index.ts b/modules/abstract-utxo/src/recovery/index.ts index 41a4acad30..d4caeb05bf 100644 --- a/modules/abstract-utxo/src/recovery/index.ts +++ b/modules/abstract-utxo/src/recovery/index.ts @@ -5,3 +5,4 @@ export * from './coingeckoApi'; export * from './crossChainRecovery'; export * from './mempoolApi'; export * from './safeRecovery'; +export * from './signExternalPsbt'; diff --git a/modules/abstract-utxo/src/recovery/signExternalPsbt.ts b/modules/abstract-utxo/src/recovery/signExternalPsbt.ts new file mode 100644 index 0000000000..02ad5ed6ce --- /dev/null +++ b/modules/abstract-utxo/src/recovery/signExternalPsbt.ts @@ -0,0 +1,134 @@ +import { BIP32, bip32, fixedScriptWallet, type CoinName } from '@bitgo/wasm-utxo'; + +import { toWasmUtxoCoinName, type UtxoCoinName } from '../names'; + +/** + * The network an externally supplied PSBT is signed on: a wasm-utxo coin + * name or an SDK UTXO coin name (normalized with toWasmUtxoCoinName). + */ +export type ExternalPsbtCoinName = CoinName | UtxoCoinName; + +/** The result of signing an externally supplied PSBT. */ +export type SignedExternalPsbt = { + /** The signed PSBT, serialized as hex. */ + psbtHex: string; + /** The input indexes that carry a valid signature by the signer key. */ + signedInputIndexes: number[]; +}; + +type PsbtOutput = { script: Uint8Array; value: bigint }; + +/** + * A key that can sign an externally supplied PSBT: a base58-encoded xprv, a + * wasm-utxo BIP32/WasmBIP32 instance, or a BIP32Interface-compatible + * (utxolib) key. Normalized with BIP32.from. + */ +export type ExternalPsbtSigner = bip32.BIP32Arg; + +function snapshotOutputs(psbt: fixedScriptWallet.BitGoPsbt): PsbtOutput[] { + return psbt.getOutputs().map(({ script, value }) => ({ script: Buffer.from(script), value })); +} + +function assertOutputsUnchanged(expected: PsbtOutput[], actual: PsbtOutput[]): void { + if ( + expected.length !== actual.length || + expected.some( + (output, index) => + output.value !== actual[index].value || !Buffer.from(output.script).equals(Buffer.from(actual[index].script)) + ) + ) { + throw new Error('PSBT outputs changed after signing'); + } +} + +/** + * Asserts the sighash policy for an externally supplied PSBT before handing it + * to a signer. + * + * Every input must commit to the entire transaction: the declared per-input + * sighash type (BIP-174 PSBT_IN_SIGHASH_TYPE) and every signature already + * present in the PSBT must be SIGHASH_ALL (or the network's full-commitment + * equivalent: SIGHASH_ALL|SIGHASH_FORKID on BCH-family coins, SIGHASH_DEFAULT + * or SIGHASH_ALL on Taproot inputs). An absent sighash type is accepted and + * uses the signer default. SIGHASH_NONE, SIGHASH_SINGLE, SIGHASH_ANYONECANPAY, + * and combinations thereof are rejected because signatures produced under + * them do not bind the signer to the transaction outputs — see WCN-1994. + * + * @param psbtHex - The externally supplied PSBT, hex-encoded + * @param coinName - The network the PSBT is signed on + * @throws Error naming the offending input if the PSBT violates the policy + */ +export function assertExternalPsbtSighashPolicy(psbtHex: string, coinName: ExternalPsbtCoinName): void { + fixedScriptWallet.BitGoPsbt.fromBytes( + Buffer.from(psbtHex, 'hex'), + toWasmUtxoCoinName(coinName) + ).assertSighashAllPolicy(); +} + +/** + * Signs an externally supplied (untrusted) PSBT with a single signer key under + * the SIGHASH_ALL-only policy. + * + * A foreign PSBT controls its own per-input sighash types, so signing it + * blindly lets the PSBT author request SIGHASH_NONE/SINGLE/ANYONECANPAY and + * produce a signature that does not bind the signer to the outputs — the + * output-swap drain demonstrated in WCN-1994. This helper closes that hole + * by enforcing, in order: + * + * 1. the sighash policy before signing (see {@link assertExternalPsbtSighashPolicy}); + * 2. that signing leaves the output set byte-identical to the snapshot taken + * before signing; + * 3. the sighash policy again on the re-parsed serialized result, so every + * signature in the exported PSBT commits to the entire transaction; + * 4. that the signer's signature cryptographically validates on the re-parsed + * result for at least one input (BitGoPsbt.sign reports every attempted + * input, including inputs the key does not match, so signed inputs are + * determined by verifying the signatures). + * + * @param psbtHex - The externally supplied PSBT, hex-encoded + * @param coinName - The network the PSBT is signed on + * @param signer - The signer key (xprv) + * @returns The signed PSBT hex and the input indexes that carry a valid + * signature by the signer key + * @throws Error if the PSBT violates the sighash policy, the outputs changed + * during signing, or the signer key produced no valid signature + */ +export function signExternalPsbt( + psbtHex: string, + coinName: ExternalPsbtCoinName, + signer: ExternalPsbtSigner +): SignedExternalPsbt { + const wasmCoinName = toWasmUtxoCoinName(coinName); + const psbt = fixedScriptWallet.BitGoPsbt.fromBytes(Buffer.from(psbtHex, 'hex'), wasmCoinName); + psbt.assertSighashAllPolicy(); + + const signerBIP32 = BIP32.from(signer); + const expectedOutputs = snapshotOutputs(psbt); + + psbt.sign(signerBIP32); + + const signedPsbtHex = Buffer.from(psbt.serialize()).toString('hex'); + // Re-parse the serialized bytes so the checks below run against exactly + // what callers will export. + const signedPsbt = fixedScriptWallet.BitGoPsbt.fromBytes(Buffer.from(signedPsbtHex, 'hex'), wasmCoinName); + + signedPsbt.assertSighashAllPolicy(); + assertOutputsUnchanged(expectedOutputs, signedPsbt.getOutputs()); + + const signerXpub = signerBIP32.neutered(); + const signedInputIndexes: number[] = []; + for (let inputIndex = 0; inputIndex < signedPsbt.inputCount(); inputIndex++) { + try { + if (signedPsbt.verifySignature(inputIndex, signerXpub)) { + signedInputIndexes.push(inputIndex); + } + } catch { + // a malformed or non-matching signature counts as not signed + } + } + if (signedInputIndexes.length === 0) { + throw new Error('No PSBT inputs were signed with the signer key'); + } + + return { psbtHex: signedPsbtHex, signedInputIndexes }; +} diff --git a/modules/abstract-utxo/test/unit/recovery/signExternalPsbt.ts b/modules/abstract-utxo/test/unit/recovery/signExternalPsbt.ts new file mode 100644 index 0000000000..46ad72bae4 --- /dev/null +++ b/modules/abstract-utxo/test/unit/recovery/signExternalPsbt.ts @@ -0,0 +1,230 @@ +import 'mocha'; +import assert from 'node:assert/strict'; + +import { fixedScriptWallet, type CoinName } from '@bitgo/wasm-utxo'; +import * as testutils from '@bitgo/wasm-utxo/testutils'; + +import { assertExternalPsbtSighashPolicy, signExternalPsbt } from '../../../src/recovery/signExternalPsbt'; + +const { BitGoPsbt, ChainCode } = fixedScriptWallet; +const { getKeyTriple } = testutils; + +const INPUT_VALUE = 100_000n; +const RECIPIENT = 'bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4'; + +const keychain = getKeyTriple('signExternalPsbt'); +const walletKeys = fixedScriptWallet.RootWalletKeys.from({ + triple: keychain, + derivationPrefixes: ['m/0/0', 'm/0/0', 'm/0/0'], +}); +const userKey = keychain[0]; +const userXprv = keychain[0].toBase58(); + +function createWalletPsbtHex(coinName: CoinName, inputCount: number, scriptType: 'p2sh' | 'p2wsh'): string { + const psbt = BitGoPsbt.createEmpty(coinName, walletKeys, { version: 2, lockTime: 0 }); + const chain = ChainCode.value(scriptType, 'external'); + for (let inputIndex = 0; inputIndex < inputCount; inputIndex++) { + psbt.addWalletInput( + { + txid: inputIndex.toString(16).padStart(2, '0').repeat(32), + vout: inputIndex, + value: INPUT_VALUE, + }, + walletKeys, + { + scriptId: { chain, index: inputIndex }, + signPath: { signer: 'user', cosigner: 'bitgo' }, + } + ); + } + return Buffer.from(psbt.serialize()).toString('hex'); +} + +function readCompactSize(bytes: Buffer, offset: number): [number, number] { + const prefix = bytes[offset]; + if (prefix < 0xfd) return [prefix, offset + 1]; + if (prefix === 0xfd) return [bytes.readUInt16LE(offset + 1), offset + 3]; + if (prefix === 0xfe) return [bytes.readUInt32LE(offset + 1), offset + 5]; + return [Number(bytes.readBigUInt64LE(offset + 1)), offset + 9]; +} + +/** + * Rewrite (or remove) the BIP-174 PSBT_IN_SIGHASH_TYPE value of a single + * input in serialized PSBT bytes, simulating a foreign PSBT crafted with an + * attacker-chosen sighash type. + */ +function rewriteInputSighashType(psbtHex: string, inputIndex: number, sighashType: number | undefined): string { + const bytes = Buffer.from(psbtHex, 'hex'); + let offset = 5; + + function readMap(targetInput: boolean): Buffer | undefined { + while (offset < bytes.length) { + const entryStart = offset; + const [keyLength, keyStart] = readCompactSize(bytes, offset); + offset = keyStart; + if (keyLength === 0) return undefined; + + const keyType = keyLength === 1 ? bytes[offset] : -1; + offset += keyLength; + const [valueLength, valueStart] = readCompactSize(bytes, offset); + offset = valueStart; + const valueEnd = valueStart + valueLength; + + if (targetInput && keyType === 0x03) { + if (sighashType === undefined) { + return Buffer.concat([bytes.subarray(0, entryStart), bytes.subarray(valueEnd)]); + } + if (valueLength !== 4) { + throw new Error('Expected a four-byte PSBT sighash value'); + } + bytes.writeUInt32LE(sighashType, valueStart); + return bytes; + } + offset = valueEnd; + } + return undefined; + } + + readMap(false); // global map + for (let index = 0; index <= inputIndex; index++) { + const rewritten = readMap(index === inputIndex); + if (rewritten) return rewritten.toString('hex'); + } + throw new Error(`No sighash type found for input ${inputIndex}`); +} + +/** + * Rewrite the sighash byte (the trailing byte) of the first PSBT_IN_PARTIAL_SIG + * value of a single input, simulating a foreign PSBT that already carries a + * signature under an unsafe sighash type. + */ +function rewritePartialSigSighashByte(psbtHex: string, inputIndex: number, sighashByte: number): string { + const bytes = Buffer.from(psbtHex, 'hex'); + let offset = 5; + + function readMap(targetInput: boolean): boolean { + while (offset < bytes.length) { + const [keyLength, keyStart] = readCompactSize(bytes, offset); + offset = keyStart; + if (keyLength === 0) return false; + + const keyType = keyLength > 1 ? bytes[offset] : -1; + offset += keyLength; + const [valueLength, valueStart] = readCompactSize(bytes, offset); + offset = valueStart; + const valueEnd = valueStart + valueLength; + + if (targetInput && keyType === 0x02) { + bytes[valueEnd - 1] = sighashByte; + return true; + } + offset = valueEnd; + } + return false; + } + + readMap(false); // global map + for (let index = 0; index <= inputIndex; index++) { + if (readMap(index === inputIndex)) { + return bytes.toString('hex'); + } + } + throw new Error(`No partial signature found for input ${inputIndex}`); +} + +describe('signExternalPsbt', function () { + it('signs every input and returns verified SIGHASH_ALL signatures', function () { + const unsignedPsbtHex = createWalletPsbtHex('btc', 2, 'p2wsh'); + const { psbtHex, signedInputIndexes } = signExternalPsbt(unsignedPsbtHex, 'btc', userXprv); + + assert.deepStrictEqual(signedInputIndexes, [0, 1]); + + const signedPsbt = BitGoPsbt.fromBytes(Buffer.from(psbtHex, 'hex'), 'btc'); + for (const inputIndex of signedInputIndexes) { + const partialSigs = signedPsbt + .getInputKeyValues(inputIndex) + .filter((keyValue) => keyValue.type === 'known' && keyValue.key === 'PSBT_IN_PARTIAL_SIG'); + assert.strictEqual(partialSigs.length, 1); + assert.strictEqual(partialSigs[0].value[partialSigs[0].value.length - 1], 0x01); + assert(signedPsbt.verifySignature(inputIndex, userKey.neutered())); + } + }); + + it('accepts BIP32 instances as the signer key', function () { + const { signedInputIndexes } = signExternalPsbt(createWalletPsbtHex('btc', 1, 'p2wsh'), 'btc', userKey); + assert.deepStrictEqual(signedInputIndexes, [0]); + }); + + const unsafeSighashModes = [ + ['SIGHASH_NONE', 0x02], + ['SIGHASH_SINGLE', 0x03], + ['SIGHASH_ANYONECANPAY', 0x80], + ['SIGHASH_ALL|ANYONECANPAY', 0x81], + ['SIGHASH_NONE|ANYONECANPAY', 0x82], + ['SIGHASH_SINGLE|ANYONECANPAY', 0x83], + ] as const; + + for (const [name, sighashType] of unsafeSighashModes) { + it(`rejects ${name} before signing`, function () { + const psbtHex = rewriteInputSighashType(createWalletPsbtHex('btc', 1, 'p2wsh'), 0, sighashType); + + assert.throws(() => assertExternalPsbtSighashPolicy(psbtHex, 'btc'), /Only SIGHASH_ALL/); + assert.throws(() => signExternalPsbt(psbtHex, 'btc', userXprv), /Only SIGHASH_ALL/); + }); + } + + it('rejects an unsafe sighash type on a later input and names it', function () { + const psbtHex = rewriteInputSighashType(createWalletPsbtHex('btc', 2, 'p2wsh'), 1, 0x03); + assert.throws(() => signExternalPsbt(psbtHex, 'btc', userXprv), /Input 1 .*Only SIGHASH_ALL/); + }); + + it('accepts an omitted sighash type, which uses the signer default', function () { + const psbtHex = rewriteInputSighashType(createWalletPsbtHex('btc', 1, 'p2wsh'), 0, undefined); + const { signedInputIndexes } = signExternalPsbt(psbtHex, 'btc', userXprv); + assert.deepStrictEqual(signedInputIndexes, [0]); + }); + + it('rejects a signer key that matches no input', function () { + const unrelatedKey = testutils.getKey('signExternalPsbt.unrelated'); + assert.throws( + () => signExternalPsbt(createWalletPsbtHex('btc', 1, 'p2wsh'), 'btc', unrelatedKey.toBase58()), + /No PSBT inputs were signed/ + ); + }); + + it('requires the FORKID form of SIGHASH_ALL on BCH-family coins', function () { + // addWalletInput stamps SIGHASH_ALL|FORKID for BCH + const bchPsbtHex = createWalletPsbtHex('bch', 1, 'p2sh'); + assert.doesNotThrow(() => assertExternalPsbtSighashPolicy(bchPsbtHex, 'bch')); + const { signedInputIndexes } = signExternalPsbt(bchPsbtHex, 'bch', userXprv); + assert.deepStrictEqual(signedInputIndexes, [0]); + + assert.throws( + () => signExternalPsbt(rewriteInputSighashType(bchPsbtHex, 0, 0x01), 'bch', userXprv), + /Only SIGHASH_ALL/ + ); + }); + + it('rejects a PSBT that already carries a non-SIGHASH_ALL signature', function () { + const { psbtHex } = signExternalPsbt(createWalletPsbtHex('btc', 1, 'p2wsh'), 'btc', userXprv); + const tampered = rewritePartialSigSighashByte(psbtHex, 0, 0x02); + + assert.throws(() => assertExternalPsbtSighashPolicy(tampered, 'btc'), /Only SIGHASH_ALL/); + assert.throws(() => signExternalPsbt(tampered, 'btc', userXprv), /Only SIGHASH_ALL/); + }); + + it('does not mutate the outputs it was given', function () { + const unsignedPsbtHex = createWalletPsbtHex('btc', 1, 'p2wsh'); + const unsignedPsbt = BitGoPsbt.fromBytes(Buffer.from(unsignedPsbtHex, 'hex'), 'btc'); + unsignedPsbt.addOutput(RECIPIENT, 90_000n); + const withOutputHex = Buffer.from(unsignedPsbt.serialize()).toString('hex'); + + const { psbtHex } = signExternalPsbt(withOutputHex, 'btc', userXprv); + const signedPsbt = BitGoPsbt.fromBytes(Buffer.from(psbtHex, 'hex'), 'btc'); + assert.strictEqual(signedPsbt.outputCount(), 1); + assert.deepStrictEqual( + signedPsbt.getOutputs().map((output) => output.value), + [90_000n] + ); + }); +}); diff --git a/modules/utxo-bin/package.json b/modules/utxo-bin/package.json index 3492bd7cf2..b656c1a939 100644 --- a/modules/utxo-bin/package.json +++ b/modules/utxo-bin/package.json @@ -31,7 +31,7 @@ "@bitgo/unspents": "^0.51.10", "@bitgo/utxo-core": "^1.41.4", "@bitgo/utxo-lib": "^11.24.4", - "@bitgo/wasm-utxo": "^5.5.1", + "@bitgo/wasm-utxo": "^5.6.0", "@noble/curves": "1.8.1", "archy": "^1.0.0", "bech32": "^2.0.0", diff --git a/modules/utxo-core/package.json b/modules/utxo-core/package.json index 3c53108d1d..d19bfcbcfe 100644 --- a/modules/utxo-core/package.json +++ b/modules/utxo-core/package.json @@ -81,7 +81,7 @@ "@bitgo/secp256k1": "^1.11.1", "@bitgo/unspents": "^0.51.10", "@bitgo/utxo-lib": "^11.24.4", - "@bitgo/wasm-utxo": "^5.5.1", + "@bitgo/wasm-utxo": "^5.6.0", "bip174": "npm:@bitgo-forks/bip174@3.1.0-master.4", "fast-sha256": "^1.3.0" }, diff --git a/modules/utxo-descriptors/package.json b/modules/utxo-descriptors/package.json index 72d0d9f7ac..24db2f6f91 100644 --- a/modules/utxo-descriptors/package.json +++ b/modules/utxo-descriptors/package.json @@ -60,7 +60,7 @@ }, "dependencies": { "@bitgo/utxo-core": "^1.41.4", - "@bitgo/wasm-utxo": "^5.5.1" + "@bitgo/wasm-utxo": "^5.6.0" }, "devDependencies": { "@stacks/bitcoin-staking": "7.6.0" diff --git a/modules/utxo-ord/package.json b/modules/utxo-ord/package.json index f01e51940a..a6ffcca007 100644 --- a/modules/utxo-ord/package.json +++ b/modules/utxo-ord/package.json @@ -45,7 +45,7 @@ "directory": "modules/utxo-ord" }, "dependencies": { - "@bitgo/wasm-utxo": "^5.5.1" + "@bitgo/wasm-utxo": "^5.6.0" }, "devDependencies": { "@bitgo/utxo-lib": "^11.24.4" diff --git a/modules/utxo-staking/package.json b/modules/utxo-staking/package.json index 65d44a3e0d..afb62fc312 100644 --- a/modules/utxo-staking/package.json +++ b/modules/utxo-staking/package.json @@ -64,7 +64,7 @@ "@bitgo/utxo-core": "^1.41.4", "@bitgo/utxo-descriptors": "^2.0.0", "@bitgo/utxo-lib": "^11.24.4", - "@bitgo/wasm-utxo": "^5.5.1", + "@bitgo/wasm-utxo": "^5.6.0", "bip174": "npm:@bitgo-forks/bip174@3.1.0-master.4", "bip322-js": "^2.0.0", "bitcoinjs-lib": "^6.1.7", diff --git a/yarn.lock b/yarn.lock index d59340a04c..73a1a08b87 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1052,10 +1052,10 @@ resolved "https://registry.npmjs.org/@bitgo/wasm-ton/-/wasm-ton-1.1.1.tgz" integrity sha512-Y4x2V2ZcYWlmx42v7dlrKDtT2DuUt8smk8E98mh7RhpiifJhLk2v5RmXDwBl0A3v9TzUOU6qMOnSS/iZ8Pq52w== -"@bitgo/wasm-utxo@^5.5.1": - version "5.5.1" - resolved "https://registry.npmjs.org/@bitgo/wasm-utxo/-/wasm-utxo-5.5.1.tgz#fab85e1067f0319c255b5f7c67d65717f73c661a" - integrity sha512-HZb8AtuqwhVcH7BRJNBBcypvxZ7a2LBnJQJE8lxnJPO2NlymiPfO8wNzf8/+UWb4mKaPk3tywzTDhocTx2+3uQ== +"@bitgo/wasm-utxo@^5.6.0": + version "5.6.0" + resolved "https://registry.npmjs.org/@bitgo/wasm-utxo/-/wasm-utxo-5.6.0.tgz#e6d09c2b8b49fcf743ebd775b7e59a6bf9f70907" + integrity sha512-OC1eehQq5fNKd7y9JIpsGpH7oHPTgA7+/Oo+K86drLWqiyPgj/P3ggFfX5S/v9KpVgfKoq0kHukd/unzjaprwg== "@brandonblack/musig@^0.0.1-alpha.0": version "0.0.1-alpha.1"