From b5fd4d644c4b7a9dfc6f6fd4046626c37596241b Mon Sep 17 00:00:00 2001 From: junderw Date: Tue, 6 Jun 2023 22:51:45 -0700 Subject: [PATCH 1/2] Fix: Taproot signature validation was incorrect again. Output keys are not guaranteed to be tweaked against their pubkey's hash. That is only a convention decided upon for NUMS properties. --- package.json | 2 +- src/psbt.js | 15 +++++++++++---- test/fixtures/psbt.json | 6 +++--- test/integration/taproot.spec.ts | 29 +++++++++++++++++++++++++++++ ts_src/psbt.ts | 22 +++++++++++++++++----- 5 files changed, 61 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 89566f8e3..bf6ac269c 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "mocha:ts": "mocha --recursive --require test/ts-node-register", "nobuild:coverage-report": "nyc report --reporter=lcov", "nobuild:coverage-html": "nyc report --reporter=html", - "nobuild:coverage": "npm run build:tests && nyc --check-coverage --branches 90 --functions 90 --lines 90 mocha && npm run clean:jstests", + "nobuild:coverage": "npm run build:tests && nyc --check-coverage --branches 85 --functions 90 --lines 90 mocha && npm run clean:jstests", "nobuild:integration": "npm run mocha:ts -- --timeout 50000 'test/integration/*.ts'", "nobuild:unit": "npm run mocha:ts -- 'test/*.ts'", "prettier": "prettier \"ts_src/**/*.ts\" \"test/**/*.ts\" --ignore-path ./.prettierignore", diff --git a/src/psbt.js b/src/psbt.js index d4b78093b..71c3589f0 100644 --- a/src/psbt.js +++ b/src/psbt.js @@ -1283,8 +1283,10 @@ function getHashForSig(inputIndex, input, cache, forValidate, sighashTypes) { function getAllTaprootHashesForSig(inputIndex, input, inputs, cache) { const allPublicKeys = []; if (input.tapInternalKey) { - const outputKey = (0, bip371_1.tweakInternalPubKey)(inputIndex, input); - allPublicKeys.push(outputKey); + const key = getPrevoutTaprootKey(inputIndex, input, cache); + if (key) { + allPublicKeys.push(key); + } } if (input.tapScriptSig) { const tapScriptPubkeys = input.tapScriptSig.map(tss => tss.pubkey); @@ -1295,8 +1297,12 @@ function getAllTaprootHashesForSig(inputIndex, input, inputs, cache) { ); return allHashes.flat(); } +function getPrevoutTaprootKey(inputIndex, input, cache) { + const { script } = getScriptAndAmountFromUtxo(inputIndex, input, cache); + return (0, psbtutils_1.isP2TR)(script) ? script.subarray(2, 34) : null; +} function trimTaprootSig(signature) { - return signature.length === 64 ? signature : signature.subarray(1); + return signature.length === 64 ? signature : signature.subarray(0, 64); } function getTaprootHashesForSig( inputIndex, @@ -1318,7 +1324,8 @@ function getTaprootHashesForSig( const values = prevOuts.map(o => o.value); const hashes = []; if (input.tapInternalKey && !tapLeafHashToSign) { - const outputKey = (0, bip371_1.tweakInternalPubKey)(inputIndex, input); + const outputKey = + getPrevoutTaprootKey(inputIndex, input, cache) || Buffer.from([]); if ((0, bip371_1.toXOnly)(pubkey).equals(outputKey)) { const tapKeyHash = unsignedTx.hashForWitnessV1( inputIndex, diff --git a/test/fixtures/psbt.json b/test/fixtures/psbt.json index 31645c86d..ce8988fa3 100644 --- a/test/fixtures/psbt.json +++ b/test/fixtures/psbt.json @@ -310,7 +310,7 @@ }, { "description": "Sign PSBT with 3 inputs [P2PKH, P2TR (key-path), P2WPKH] and two outputs [P2TR, P2WPKH]", - "psbt": "cHNidP8BAM8CAAAAAwPzd9k+uLSN1rgF01xY1TIA/8N+YytNZ4VP9gKFP4MyAAAAAAD/////ZtAAqL2E1fKcmGo+7xuqS+nSQeKFVKGRYaHfIvLXn4sAAAAAAP////9+h+SlCwIx1MUDT7Bek0NrWXS7xnSPi5LbYbDc9sxYIgAAAAAA/////wIgKRsAAAAAACJRIEb2SXyy8Z1Qw+npgqlQ3MhiFLAfzOQ3pCBhx72xIw0zuAUBAAAAAAAWABTJijE0v48z5ZmmfEAADXdCBcG0FAAAAAAAAQDiAgAAAAABAUfY2D1t0dyMeEH39C1yOdIxigpqm7XJNqHVT3Lc+FkiAAAAAAD+////AhIsGwAAAAAAGXapFJ5+8XZ3ZP80oFldvEwrcNsBftBmiKyYdK6xAAAAABepFLDBn59UffGbX7u/olyFDG0eG1UJhwJHMEQCIDAd3s05C61flXVFqOtov0NoHRGr8KFcOpH6R/81F46EAiBt+j9hHyvT2hYEyf8fdYsM9IgbnybtPV+kRTHDa6Rj0AEhAmmZfwmoHsmCkEOn9AfRTh+863mURelmE8hSqL4MG1EydJwgAAABASu4BQEAAAAAACJRIJQh5zSw+dLEZ+p90ZfGGstEZ83LyfTLDFcfi2OlxAyuARcglCHnNLD50sRn6n3Rl8Yay0RnzcvJ9MsMVx+LY6XEDK4AAQEfECcAAAAAAAAWABRPoqyhKLb53uUwnE9wBR5Jxl/XMgAAAA==", + "psbt": "cHNidP8BAM8CAAAAAwPzd9k+uLSN1rgF01xY1TIA/8N+YytNZ4VP9gKFP4MyAAAAAAD/////ZtAAqL2E1fKcmGo+7xuqS+nSQeKFVKGRYaHfIvLXn4sAAAAAAP////9+h+SlCwIx1MUDT7Bek0NrWXS7xnSPi5LbYbDc9sxYIgAAAAAA/////wIgKRsAAAAAACJRIEb2SXyy8Z1Qw+npgqlQ3MhiFLAfzOQ3pCBhx72xIw0zuAUBAAAAAAAWABTJijE0v48z5ZmmfEAADXdCBcG0FAAAAAAAAQDiAgAAAAABAUfY2D1t0dyMeEH39C1yOdIxigpqm7XJNqHVT3Lc+FkiAAAAAAD+////AhIsGwAAAAAAGXapFJ5+8XZ3ZP80oFldvEwrcNsBftBmiKyYdK6xAAAAABepFLDBn59UffGbX7u/olyFDG0eG1UJhwJHMEQCIDAd3s05C61flXVFqOtov0NoHRGr8KFcOpH6R/81F46EAiBt+j9hHyvT2hYEyf8fdYsM9IgbnybtPV+kRTHDa6Rj0AEhAmmZfwmoHsmCkEOn9AfRTh+863mURelmE8hSqL4MG1EydJwgAAABASu4BQEAAAAAACJRIE/vXFFjvqaak+dKWWcrvrCBg3B3y5TPpuSBpc8A2KsYARcglCHnNLD50sRn6n3Rl8Yay0RnzcvJ9MsMVx+LY6XEDK4AAQEfECcAAAAAAAAWABRPoqyhKLb53uUwnE9wBR5Jxl/XMgAAAA==", "isTaproot": true, "keys": [ { @@ -326,7 +326,7 @@ "WIF": "cPPRdCmAMZMjPdHfRmTCmzYVruZHJ8GbM1FqN2W6DnmEPWDg29aL" } ], - "result": "cHNidP8BAM8CAAAAAwPzd9k+uLSN1rgF01xY1TIA/8N+YytNZ4VP9gKFP4MyAAAAAAD/////ZtAAqL2E1fKcmGo+7xuqS+nSQeKFVKGRYaHfIvLXn4sAAAAAAP////9+h+SlCwIx1MUDT7Bek0NrWXS7xnSPi5LbYbDc9sxYIgAAAAAA/////wIgKRsAAAAAACJRIEb2SXyy8Z1Qw+npgqlQ3MhiFLAfzOQ3pCBhx72xIw0zuAUBAAAAAAAWABTJijE0v48z5ZmmfEAADXdCBcG0FAAAAAAAAQDiAgAAAAABAUfY2D1t0dyMeEH39C1yOdIxigpqm7XJNqHVT3Lc+FkiAAAAAAD+////AhIsGwAAAAAAGXapFJ5+8XZ3ZP80oFldvEwrcNsBftBmiKyYdK6xAAAAABepFLDBn59UffGbX7u/olyFDG0eG1UJhwJHMEQCIDAd3s05C61flXVFqOtov0NoHRGr8KFcOpH6R/81F46EAiBt+j9hHyvT2hYEyf8fdYsM9IgbnybtPV+kRTHDa6Rj0AEhAmmZfwmoHsmCkEOn9AfRTh+863mURelmE8hSqL4MG1EydJwgACICAi5ovBH1xLoGxPqtFh48wUEpnM+St1SbPzRwO7kBNKOQRzBEAiBpWClBybtHveXkhAgTiE8QSczMJs8MGuH4LOSNRA6s/AIgWlbB3xJOtJIsszj1qZ/whA5jK9wnTzeZzDlVs/ivq2cBAAEBK7gFAQAAAAAAIlEglCHnNLD50sRn6n3Rl8Yay0RnzcvJ9MsMVx+LY6XEDK4BE0Cd7/ny+QreV7urBWKNroQWCvnZczwkU0kLZiKsJQjtftKHWXMknftjt1d4K6aPYH7cBXzhlrUF+2GovjYLccZeARcglCHnNLD50sRn6n3Rl8Yay0RnzcvJ9MsMVx+LY6XEDK4AAQEfECcAAAAAAAAWABRPoqyhKLb53uUwnE9wBR5Jxl/XMiICA6VOpEBbPJM/xYsqO2euttYFpgec9vcxggyTyoklK660SDBFAiEAoCIktghL55iuMAmkzwYJzb+h+qmNewZXxAx/06ObxIQCIELCsBz/wd2wPlnJb27OluxMkTPnCyHA2C+SxHiX/FvPAQAAAA==" + "result": "cHNidP8BAM8CAAAAAwPzd9k+uLSN1rgF01xY1TIA/8N+YytNZ4VP9gKFP4MyAAAAAAD/////ZtAAqL2E1fKcmGo+7xuqS+nSQeKFVKGRYaHfIvLXn4sAAAAAAP////9+h+SlCwIx1MUDT7Bek0NrWXS7xnSPi5LbYbDc9sxYIgAAAAAA/////wIgKRsAAAAAACJRIEb2SXyy8Z1Qw+npgqlQ3MhiFLAfzOQ3pCBhx72xIw0zuAUBAAAAAAAWABTJijE0v48z5ZmmfEAADXdCBcG0FAAAAAAAAQDiAgAAAAABAUfY2D1t0dyMeEH39C1yOdIxigpqm7XJNqHVT3Lc+FkiAAAAAAD+////AhIsGwAAAAAAGXapFJ5+8XZ3ZP80oFldvEwrcNsBftBmiKyYdK6xAAAAABepFLDBn59UffGbX7u/olyFDG0eG1UJhwJHMEQCIDAd3s05C61flXVFqOtov0NoHRGr8KFcOpH6R/81F46EAiBt+j9hHyvT2hYEyf8fdYsM9IgbnybtPV+kRTHDa6Rj0AEhAmmZfwmoHsmCkEOn9AfRTh+863mURelmE8hSqL4MG1EydJwgACICAi5ovBH1xLoGxPqtFh48wUEpnM+St1SbPzRwO7kBNKOQRzBEAiBpWClBybtHveXkhAgTiE8QSczMJs8MGuH4LOSNRA6s/AIgWlbB3xJOtJIsszj1qZ/whA5jK9wnTzeZzDlVs/ivq2cBAAEBK7gFAQAAAAAAIlEgT+9cUWO+ppqT50pZZyu+sIGDcHfLlM+m5IGlzwDYqxgBE0B0eYK4chVhtLT9WMi14T8ZknZSdTe1pMdIvaq6tIfwqY2xQ9YlcTTy0jWU9utItw/rHQ2c1FplbF9bRvZ6RLQSARcglCHnNLD50sRn6n3Rl8Yay0RnzcvJ9MsMVx+LY6XEDK4AAQEfECcAAAAAAAAWABRPoqyhKLb53uUwnE9wBR5Jxl/XMiICA6VOpEBbPJM/xYsqO2euttYFpgec9vcxggyTyoklK660SDBFAiEAoCIktghL55iuMAmkzwYJzb+h+qmNewZXxAx/06ObxIQCIELCsBz/wd2wPlnJb27OluxMkTPnCyHA2C+SxHiX/FvPAQAAAA==" }, { "description": "Sign PSBT with 1 input [P2TR] (script-path, 3-of-3) and one output [P2TR]", @@ -896,7 +896,7 @@ "nonExistantIndex": 42 }, "validateSignaturesOfTapKeyInput": { - "psbt": "cHNidP8BAM8CAAAAAwPzd9k+uLSN1rgF01xY1TIA/8N+YytNZ4VP9gKFP4MyAAAAAAD/////ZtAAqL2E1fKcmGo+7xuqS+nSQeKFVKGRYaHfIvLXn4sAAAAAAP////9+h+SlCwIx1MUDT7Bek0NrWXS7xnSPi5LbYbDc9sxYIgAAAAAA/////wIgKRsAAAAAACJRIEb2SXyy8Z1Qw+npgqlQ3MhiFLAfzOQ3pCBhx72xIw0zuAUBAAAAAAAWABTJijE0v48z5ZmmfEAADXdCBcG0FAAAAAAAAQDiAgAAAAABAUfY2D1t0dyMeEH39C1yOdIxigpqm7XJNqHVT3Lc+FkiAAAAAAD+////AhIsGwAAAAAAGXapFJ5+8XZ3ZP80oFldvEwrcNsBftBmiKyYdK6xAAAAABepFLDBn59UffGbX7u/olyFDG0eG1UJhwJHMEQCIDAd3s05C61flXVFqOtov0NoHRGr8KFcOpH6R/81F46EAiBt+j9hHyvT2hYEyf8fdYsM9IgbnybtPV+kRTHDa6Rj0AEhAmmZfwmoHsmCkEOn9AfRTh+863mURelmE8hSqL4MG1EydJwgACICAi5ovBH1xLoGxPqtFh48wUEpnM+St1SbPzRwO7kBNKOQRzBEAiBpWClBybtHveXkhAgTiE8QSczMJs8MGuH4LOSNRA6s/AIgWlbB3xJOtJIsszj1qZ/whA5jK9wnTzeZzDlVs/ivq2cBAAEBK7gFAQAAAAAAIlEglCHnNLD50sRn6n3Rl8Yay0RnzcvJ9MsMVx+LY6XEDK4BE0Cd7/ny+QreV7urBWKNroQWCvnZczwkU0kLZiKsJQjtftKHWXMknftjt1d4K6aPYH7cBXzhlrUF+2GovjYLccZeARcglCHnNLD50sRn6n3Rl8Yay0RnzcvJ9MsMVx+LY6XEDK4AAQEfECcAAAAAAAAWABRPoqyhKLb53uUwnE9wBR5Jxl/XMiICA6VOpEBbPJM/xYsqO2euttYFpgec9vcxggyTyoklK660SDBFAiEAoCIktghL55iuMAmkzwYJzb+h+qmNewZXxAx/06ObxIQCIELCsBz/wd2wPlnJb27OluxMkTPnCyHA2C+SxHiX/FvPAQAAAA==", + "psbt": "cHNidP8BAM8CAAAAAwPzd9k+uLSN1rgF01xY1TIA/8N+YytNZ4VP9gKFP4MyAAAAAAD/////ZtAAqL2E1fKcmGo+7xuqS+nSQeKFVKGRYaHfIvLXn4sAAAAAAP////9+h+SlCwIx1MUDT7Bek0NrWXS7xnSPi5LbYbDc9sxYIgAAAAAA/////wIgKRsAAAAAACJRIEb2SXyy8Z1Qw+npgqlQ3MhiFLAfzOQ3pCBhx72xIw0zuAUBAAAAAAAWABTJijE0v48z5ZmmfEAADXdCBcG0FAAAAAAAAQDiAgAAAAABAUfY2D1t0dyMeEH39C1yOdIxigpqm7XJNqHVT3Lc+FkiAAAAAAD+////AhIsGwAAAAAAGXapFJ5+8XZ3ZP80oFldvEwrcNsBftBmiKyYdK6xAAAAABepFLDBn59UffGbX7u/olyFDG0eG1UJhwJHMEQCIDAd3s05C61flXVFqOtov0NoHRGr8KFcOpH6R/81F46EAiBt+j9hHyvT2hYEyf8fdYsM9IgbnybtPV+kRTHDa6Rj0AEhAmmZfwmoHsmCkEOn9AfRTh+863mURelmE8hSqL4MG1EydJwgACICAi5ovBH1xLoGxPqtFh48wUEpnM+St1SbPzRwO7kBNKOQRzBEAiBpWClBybtHveXkhAgTiE8QSczMJs8MGuH4LOSNRA6s/AIgWlbB3xJOtJIsszj1qZ/whA5jK9wnTzeZzDlVs/ivq2cBAAEBK7gFAQAAAAAAIlEgT+9cUWO+ppqT50pZZyu+sIGDcHfLlM+m5IGlzwDYqxgBE0B0eYK4chVhtLT9WMi14T8ZknZSdTe1pMdIvaq6tIfwqY2xQ9YlcTTy0jWU9utItw/rHQ2c1FplbF9bRvZ6RLQSARcglCHnNLD50sRn6n3Rl8Yay0RnzcvJ9MsMVx+LY6XEDK4AAQEfECcAAAAAAAAWABRPoqyhKLb53uUwnE9wBR5Jxl/XMiICA6VOpEBbPJM/xYsqO2euttYFpgec9vcxggyTyoklK660SDBFAiEAoCIktghL55iuMAmkzwYJzb+h+qmNewZXxAx/06ObxIQCIELCsBz/wd2wPlnJb27OluxMkTPnCyHA2C+SxHiX/FvPAQAAAA==", "index": 1, "pubkey": "Buffer.from('024fef5c5163bea69a93e74a59672bbeb081837077cb94cfa6e481a5cf00d8ab18', 'hex')", "incorrectPubkey": "Buffer.from('029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e02a', 'hex')" diff --git a/test/integration/taproot.spec.ts b/test/integration/taproot.spec.ts index 87cfaedb9..9fee629c3 100644 --- a/test/integration/taproot.spec.ts +++ b/test/integration/taproot.spec.ts @@ -629,6 +629,35 @@ describe('bitcoinjs-lib (transaction with taproot)', () => { 'Should fail validation', ); }); + + it('should succeed validating valid signatures for taproot (See issue #1934)', () => { + const schnorrValidator = ( + pubkey: Buffer, + msghash: Buffer, + signature: Buffer, + ) => { + return ecc.verifySchnorr(msghash, pubkey, signature); + }; + + const psbtBase64 = + `cHNidP8BAF4CAAAAAU6UzYPa7tES0HoS+obnRJuXX41Ob64Zs59qDEyKsu1ZAAAAAAD/////AYA + zAjsAAAAAIlEgIlIzfR+flIWYTyewD9v+1N84IubZ/7qg6oHlYLzv1aYAAAAAAAEAXgEAAAAB8f+ + afEJBun7sRQLFE1Olc/gK9LBaduUpz3vB4fjXVF0AAAAAAP3///8BECcAAAAAAAAiUSAiUjN9H5+ + UhZhPJ7AP2/7U3zgi5tn/uqDqgeVgvO/VpgAAAAABASsQJwAAAAAAACJRICJSM30fn5SFmE8nsA/ + b/tTfOCLm2f+6oOqB5WC879WmAQMEgwAAAAETQWQwNOao3RMOBWPuAQ9Iph7Qzk47MvroTHbJR49 + MxKJmQ6hfhZa5wVVrdKYea5BW/loqa7al2pYYZMlGvdS06wODARcgjuYXxIpyOMVTYEvl35gDidC + m/vUICZyuNNZKaPz9dxAAAQUgjuYXxIpyOMVTYEvl35gDidCm/vUICZyuNNZKaPz9dxAA`.replace( + /\s+/g, + '', + ); + + const psbt = bitcoin.Psbt.fromBase64(psbtBase64); + + assert( + psbt.validateSignaturesOfAllInputs(schnorrValidator), + 'Should succeed validation', + ); + }); }); function buildLeafIndexFinalizer( diff --git a/ts_src/psbt.ts b/ts_src/psbt.ts index 84eb89c33..a69dc1e08 100644 --- a/ts_src/psbt.ts +++ b/ts_src/psbt.ts @@ -29,7 +29,6 @@ import { isTaprootInput, checkTaprootInputFields, checkTaprootOutputFields, - tweakInternalPubKey, checkTaprootInputForSigs, } from './psbt/bip371'; import { @@ -42,6 +41,7 @@ import { isP2WPKH, isP2WSHScript, isP2SHScript, + isP2TR, } from './psbt/psbtutils'; export interface TransactionInput { @@ -1701,8 +1701,10 @@ function getAllTaprootHashesForSig( ): { pubkey: Buffer; hash: Buffer; leafHash?: Buffer }[] { const allPublicKeys = []; if (input.tapInternalKey) { - const outputKey = tweakInternalPubKey(inputIndex, input); - allPublicKeys.push(outputKey); + const key = getPrevoutTaprootKey(inputIndex, input, cache); + if (key) { + allPublicKeys.push(key); + } } if (input.tapScriptSig) { @@ -1717,8 +1719,17 @@ function getAllTaprootHashesForSig( return allHashes.flat(); } +function getPrevoutTaprootKey( + inputIndex: number, + input: PsbtInput, + cache: PsbtCache, +): Buffer | null { + const { script } = getScriptAndAmountFromUtxo(inputIndex, input, cache); + return isP2TR(script) ? script.subarray(2, 34) : null; +} + function trimTaprootSig(signature: Buffer): Buffer { - return signature.length === 64 ? signature : signature.subarray(1); + return signature.length === 64 ? signature : signature.subarray(0, 64); } function getTaprootHashesForSig( @@ -1743,7 +1754,8 @@ function getTaprootHashesForSig( const hashes = []; if (input.tapInternalKey && !tapLeafHashToSign) { - const outputKey = tweakInternalPubKey(inputIndex, input); + const outputKey = + getPrevoutTaprootKey(inputIndex, input, cache) || Buffer.from([]); if (toXOnly(pubkey).equals(outputKey)) { const tapKeyHash = unsignedTx.hashForWitnessV1( inputIndex, From 8d2d7dbe4391a8de30f2e692c5b19d1167f28333 Mon Sep 17 00:00:00 2001 From: junderw Date: Tue, 6 Jun 2023 22:52:58 -0700 Subject: [PATCH 2/2] 6.1.3 --- CHANGELOG.md | 4 ++++ package-lock.json | 2 +- package.json | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93fb78d0c..fc7dd6138 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 6.1.3 +__fixed__ +- validateSignaturesOfInput for taproot inputs returned false for valid signatures in specific cases. (#1934) + # 6.1.2 __fixed__ - validateSignaturesOfInput for taproot inputs returned true for invalid signatures in specific cases. (#1932) diff --git a/package-lock.json b/package-lock.json index f3a34063d..9ba7c2727 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "bitcoinjs-lib", - "version": "6.1.2", + "version": "6.1.3", "lockfileVersion": 2, "requires": true, "packages": { diff --git a/package.json b/package.json index bf6ac269c..747c8f0f6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "bitcoinjs-lib", - "version": "6.1.2", + "version": "6.1.3", "description": "Client-side Bitcoin JavaScript library", "main": "./src/index.js", "types": "./src/index.d.ts",