fix(core): Stricter Bitcoin transaction checks.

core/.changelog.d/noissue.security

@@ -0,0 +1 @@
+Fix a coin loss vulnerability related to replacement transactions with multisig inputs and unverified external inputs.

core/src/apps/bitcoin/sign_tx/tx_info.py

@@ -153,8 +153,11 @@ class OriginalTxInfo(TxInfoBase):
         super().add_input(txi, script_pubkey)
         writers.write_tx_input(self.h_tx, txi, txi.script_sig or bytes())
 
-        # For verification use the first original input that specifies address_n.
-        if not self.verification_input and txi.address_n:
+        # For verification use the first original non-multisig input that specifies address_n.
+        # NOTE: Supporting replacement transactions where all internal inputs are multisig would
+        # require checking the signatures of all of the original internal inputs or not allowing
+        # unverified external inputs in transactions where multisig inputs are present.
+        if not self.verification_input and txi.address_n and not txi.multisig:
             self.verification_input = txi
             self.verification_index = self.index
 

core/src/apps/bitcoin/writers.py

@@ -15,8 +15,6 @@ from apps.common.writers import (  # noqa: F401
     write_uint64_le,
 )
 
-from .common import input_is_external_unverified
-
 if TYPE_CHECKING:
     from trezor.messages import (
         PrevInput,
@@ -48,15 +46,23 @@ def write_tx_input(w: Writer, i: TxInput | PrevInput, script: bytes) -> None:
 
 
 def write_tx_input_check(w: Writer, i: TxInput) -> None:
-    write_bytes_fixed(w, i.prev_hash, TX_HASH_SIZE)
-    write_uint32(w, i.prev_index)
-    write_uint32(w, i.script_type)
-    write_uint8(w, input_is_external_unverified(i))
+    from .multisig import multisig_fingerprint
+
     write_uint32(w, len(i.address_n))
     for n in i.address_n:
         write_uint32(w, n)
+    write_bytes_fixed(w, i.prev_hash, TX_HASH_SIZE)
+    write_uint32(w, i.prev_index)
+    write_bytes_prefixed(w, i.script_sig or b"")
     write_uint32(w, i.sequence)
+    write_uint32(w, i.script_type)
+    multisig_fp = multisig_fingerprint(i.multisig) if i.multisig else b""
+    write_bytes_prefixed(w, multisig_fp)
     write_uint64(w, i.amount or 0)
+    write_bytes_prefixed(w, i.witness or b"")
+    write_bytes_prefixed(w, i.ownership_proof or b"")
+    write_bytes_prefixed(w, i.orig_hash or b"")
+    write_uint32(w, i.orig_index or 0)
     write_bytes_prefixed(w, i.script_pubkey or b"")
 
 

core/tests/test_apps.bitcoin.sign_tx.writers.py

@@ -43,7 +43,7 @@ class TestWriters(unittest.TestCase):
 
         b = bytearray()
         writers.write_tx_input_check(b, inp)
-        self.assertEqual(len(b), 32 + 4 + 4 + 1 + 4 + 4 + 4 + 8 + 26)
+        self.assertEqual(len(b), 4 + 4 + 32 + 4 + 11 + 4 + 4 + 1 + 8 + 1 + 1 + 1 + 4 + 26)
 
         for bad_prevhash in (b"", b"x", b"hello", b"x" * 33):
             inp.prev_hash = bad_prevhash