From eb16e8e1bb93e7a8cc9182399b0d61fdf13259b1 Mon Sep 17 00:00:00 2001
From: Otto Allmendinger <otto@bitgo.com>
Date: Wed, 25 Oct 2023 17:41:59 +0200
Subject: [PATCH] fix(utxo-lib): check prevout script against computed for
 non-taproot inputs

Make sure that we are actually signing the right script.

Taproot cases will follow.

Issue: BTC-428
---
 modules/utxo-lib/src/bitgo/outputScripts.ts   | 30 ++++++++++++++-----
 modules/utxo-lib/src/bitgo/signature.ts       | 16 ++++++++++
 modules/utxo-lib/test/bitgo/signature.ts      | 12 +++++++-
 .../utxo-lib/test/bitgo/signatureModify.ts    | 17 ++++++++++-
 4 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/modules/utxo-lib/src/bitgo/outputScripts.ts b/modules/utxo-lib/src/bitgo/outputScripts.ts
index 3ef58233ce..f9639a1169 100644
--- a/modules/utxo-lib/src/bitgo/outputScripts.ts
+++ b/modules/utxo-lib/src/bitgo/outputScripts.ts
@@ -111,6 +111,27 @@ export function createOutputScriptP2shP2pk(pubkey: Buffer): SpendableScript {
   };
 }
 
+export function getOutputScript(scriptType: 'p2sh' | 'p2shP2wsh' | 'p2wsh', conditionScript: Buffer): Buffer {
+  let output;
+  switch (scriptType) {
+    case 'p2sh':
+      ({ output } = bitcoinjs.payments.p2sh({ redeem: { output: conditionScript } }));
+      break;
+    case 'p2shP2wsh':
+      ({ output } = bitcoinjs.payments.p2sh({
+        redeem: { output: getOutputScript('p2wsh', conditionScript) },
+      }));
+      break;
+    case 'p2wsh':
+      ({ output } = bitcoinjs.payments.p2wsh({ redeem: { output: conditionScript } }));
+      break;
+  }
+  if (output === undefined) {
+    throw new Error(`output undefined`);
+  }
+  return output;
+}
+
 /**
  * Return scripts for 2-of-3 multisig output
  * @param pubkeys - the key triple for multisig
@@ -148,32 +169,25 @@ export function createOutputScript2of3(
   const script2of3 = bitcoinjs.payments.p2ms({ m: 2, pubkeys });
   assert(script2of3.output);
 
-  let scriptPubKey: bitcoinjs.Payment;
   let redeemScript: bitcoinjs.Payment | undefined;
   let witnessScript: bitcoinjs.Payment | undefined;
   switch (scriptType) {
     case 'p2sh':
       redeemScript = script2of3;
-      scriptPubKey = bitcoinjs.payments.p2sh({ redeem: script2of3 });
       break;
     case 'p2shP2wsh':
       witnessScript = script2of3;
       redeemScript = bitcoinjs.payments.p2wsh({ redeem: script2of3 });
-      scriptPubKey = bitcoinjs.payments.p2sh({ redeem: redeemScript });
       break;
     case 'p2wsh':
       witnessScript = script2of3;
-      scriptPubKey = bitcoinjs.payments.p2wsh({ redeem: witnessScript });
       break;
     default:
       throw new Error(`unknown multisig script type ${scriptType}`);
   }
 
-  assert(scriptPubKey);
-  assert(scriptPubKey.output);
-
   return {
-    scriptPubKey: scriptPubKey.output,
+    scriptPubKey: getOutputScript(scriptType, script2of3.output),
     redeemScript: redeemScript?.output,
     witnessScript: witnessScript?.output,
   };
diff --git a/modules/utxo-lib/src/bitgo/signature.ts b/modules/utxo-lib/src/bitgo/signature.ts
index eb766209b5..92cb514895 100644
--- a/modules/utxo-lib/src/bitgo/signature.ts
+++ b/modules/utxo-lib/src/bitgo/signature.ts
@@ -8,6 +8,7 @@ import {
   createOutputScript2of3,
   createOutputScriptP2shP2pk,
   createSpendScriptP2tr,
+  getOutputScript,
   ScriptType,
   ScriptType2Of3,
   scriptType2Of3AsPrevOutType,
@@ -102,6 +103,21 @@ export function getSignatureVerifications<TNumber extends number | bigint>(
     }
   }
 
+  if (
+    parsedScript.scriptType !== 'taprootKeyPathSpend' &&
+    parsedScript.scriptType !== 'taprootScriptPathSpend' &&
+    prevOutputs
+  ) {
+    const prevoutScript = prevOutputs[inputIndex].script;
+
+    const output = getOutputScript(parsedScript.scriptType, parsedScript.pubScript);
+    if (!prevoutScript.equals(output)) {
+      throw new Error(
+        `prevout script ${prevoutScript.toString('hex')} does not match computed script ${output.toString('hex')}`
+      );
+    }
+  }
+
   let publicKeys: Buffer[];
   if (parsedScript.scriptType === 'taprootKeyPathSpend') {
     if (!prevOutputs) {
diff --git a/modules/utxo-lib/test/bitgo/signature.ts b/modules/utxo-lib/test/bitgo/signature.ts
index d6bb2460b7..5cba5b2e0b 100644
--- a/modules/utxo-lib/test/bitgo/signature.ts
+++ b/modules/utxo-lib/test/bitgo/signature.ts
@@ -29,7 +29,7 @@ import {
   getSignKeyCombinations,
   getUnsignedTransaction2Of3,
 } from '../transaction_util';
-import { getTransactionWithHighS } from './signatureModify';
+import { getPrevOutsWithInvalidOutputScript, getTransactionWithHighS } from './signatureModify';
 import { normDefault } from '../testutil/normalize';
 
 function getScriptTypes2Of3() {
@@ -291,6 +291,16 @@ function checkSignTransaction<TNumber extends number | bigint>(
           signKeys.length - 1
         );
       });
+
+      if (scriptType !== 'p2tr' && scriptType !== 'p2trMusig2') {
+        assert.throws(
+          () =>
+            signatureCount(
+              verifySignatureWithPublicKeys<TNumber>(tx, i, getPrevOutsWithInvalidOutputScript(prevOutputs, i), pubkeys)
+            ),
+          /prevout script .* does not match computed script .*/
+        );
+      }
     }
   });
 }
diff --git a/modules/utxo-lib/test/bitgo/signatureModify.ts b/modules/utxo-lib/test/bitgo/signatureModify.ts
index b6458a9673..984f2b28c5 100644
--- a/modules/utxo-lib/test/bitgo/signatureModify.ts
+++ b/modules/utxo-lib/test/bitgo/signatureModify.ts
@@ -1,5 +1,5 @@
 /* eslint-disable no-redeclare */
-import { script, ScriptSignature } from 'bitcoinjs-lib';
+import { script, ScriptSignature, TxOutput } from 'bitcoinjs-lib';
 import { isPlaceholderSignature, parseSignatureScript, UtxoTransaction } from '../../src/bitgo';
 
 const BN = require('bn.js');
@@ -79,3 +79,18 @@ export function getTransactionWithHighS<TNumber extends number | bigint>(
     return [cloned];
   });
 }
+
+/** Return transaction with script xored with 0xff for the given input */
+export function getPrevOutsWithInvalidOutputScript<TNumber extends number | bigint>(
+  prevOuts: TxOutput<TNumber>[],
+  inputIndex: number
+): TxOutput<TNumber>[] {
+  return prevOuts.map((prevOut, i) => {
+    return i === inputIndex
+      ? {
+          ...prevOut,
+          script: prevOut.script.map((v) => v ^ 0xff) as typeof prevOut.script,
+        }
+      : prevOut;
+  });
+}