fix(core/bitcoin): Fix external input misidentification.
What the developer wrote
Authored by Andrew Kozlik
fix(core/bitcoin): Fix external input misidentification.
(cherry picked from commit 23bbf680071f7ede286dc3eda2f4baa7193288b2)
What changed, and why it matters
This update fixes a bug in the Trezor hardware wallet's Bitcoin transaction signing. When a transaction included an external input (someone else's coin) with empty placeholder fields, the device could mistake it for an already-signed input. That would let it skip both signature verification and the warning that tells the user the input is unverified. The fix makes the device treat empty bytes the same as missing fields and rejects ownership proofs on internal or already-signed inputs.
Apply the patch and run the updated Bitcoin signing tests, especially `test_signtx_external.py`. Review any custom integrations that construct external inputs with empty `witness` or `script_sig` fields to ensure they rely on the corrected unverified-external behavior.
Security signals we found
Security-relevant bug fix in Bitcoin transaction signing
Change from identity comparison to truthiness to handle empty byte fields consistently
Addition of input sanitization rules for ownership_proof presence
New regression test for empty witness/script_sig external inputs
Changelog entry labeled as security fix
Evidence from the diff
The patch changes input_is_external_unverified() in core/src/apps/bitcoin/common.py to evaluate ownership_proof, witness, and script_sig as booleans rather than comparing them to None. In Protobuf-generated message objects, empty bytes fields are b"", not None, so the old check treated witness=b"" / script_sig=b"" as absent and classified the input as unverified-external. The new check treats empty bytes as falsy, matching process_external_input(). Additionally, _sanitize_tx_input() now rejects ownership_proof on internal inputs and rejects ownership_proof when witness or script_sig is present, preventing contradictory input states. A new device test confirms that an external P2TR input with empty witness/script_sig still triggers the external-input failure.
Changed components
core/src/apps/bitcoin/common.pycore/src/apps/bitcoin/sign_tx/helpers.pycore/tests/test_apps.bitcoin.approver.pytests/device_tests/bitcoin/test_signtx_external.pyInspect captured patch +55 / −4
diff --git a/core/.changelog.d/+external.security b/core/.changelog.d/+external.security
new file mode 100644
index 00000000..082be74c
--- /dev/null
+++ b/core/.changelog.d/+external.security
@@ -0,0 +1 @@
+Fix external input misidentification in bitcoin signing.
diff --git a/core/src/apps/bitcoin/common.py b/core/src/apps/bitcoin/common.py
index b930a2c1..bd1aebc9 100644
--- a/core/src/apps/bitcoin/common.py
+++ b/core/src/apps/bitcoin/common.py
@@ -183,11 +183,12 @@ def input_is_external(txi: TxInput) -> bool:
def input_is_external_unverified(txi: TxInput) -> bool:
+ # Evaluate fields as bool, same as in `process_external_input()` for consistency in case of empty bytes.
return (
txi.script_type == InputScriptType.EXTERNAL
- and txi.ownership_proof is None
- and txi.witness is None
- and txi.script_sig is None
+ and not txi.ownership_proof
+ and not txi.witness
+ and not txi.script_sig
)
diff --git a/core/src/apps/bitcoin/sign_tx/helpers.py b/core/src/apps/bitcoin/sign_tx/helpers.py
index b3ad39f7..c0b2c98e 100644
--- a/core/src/apps/bitcoin/sign_tx/helpers.py
+++ b/core/src/apps/bitcoin/sign_tx/helpers.py
@@ -501,6 +501,9 @@ def _sanitize_tx_input(txi: TxInput, coin: CoinInfo) -> TxInput:
if txi.script_pubkey:
raise DataError("Input's script_pubkey provided but not expected.")
+
+ if txi.ownership_proof:
+ raise DataError("Ownership proof not expected for internal input.")
else:
if txi.address_n:
raise DataError("Input's address_n provided but not expected.")
@@ -518,6 +521,9 @@ def _sanitize_tx_input(txi: TxInput, coin: CoinInfo) -> TxInput:
if script_type == InputScriptType.SPENDTAPROOT and not coin.taproot:
raise DataError("Taproot not enabled on this coin")
+ if txi.ownership_proof and (txi.witness or txi.script_sig):
+ raise DataError("Ownership proof not expected for presigned input.")
+
if txi.commitment_data and not txi.ownership_proof:
raise DataError("commitment_data field provided but not expected.")
diff --git a/core/tests/test_apps.bitcoin.approver.py b/core/tests/test_apps.bitcoin.approver.py
index a31fd370..f7069d10 100644
--- a/core/tests/test_apps.bitcoin.approver.py
+++ b/core/tests/test_apps.bitcoin.approver.py
@@ -73,7 +73,7 @@ class TestApprover(TestCaseWithContext):
script_pubkey=bytes(22),
script_type=InputScriptType.EXTERNAL,
sequence=0xFFFFFFFF,
- witness="",
+ witness=bytes(1),
)
for i in range(99)
]
diff --git a/tests/device_tests/bitcoin/test_signtx_external.py b/tests/device_tests/bitcoin/test_signtx_external.py
index 3e6e2e4d..0e20a14f 100644
--- a/tests/device_tests/bitcoin/test_signtx_external.py
+++ b/tests/device_tests/bitcoin/test_signtx_external.py
@@ -842,6 +842,49 @@ def test_p2tr_external_unverified(session: Session):
)
+def test_p2tr_external_unverified_empty_witness(session: Session):
+ # An external input with witness=b"" / script_sig=b"" must be treated
+ # the same as one with the fields absent — i.e. unverified. If empty
+ # bytes were accepted as a "presigned" witness, the input would skip
+ # both signature verification and the unverified-external warning.
+ inp1 = messages.TxInputType(
+ # tb1pswrqtykue8r89t9u4rprjs0gt4qzkdfuursfnvqaa3f2yql07zmq8s8a5u
+ address_n=parse_path("m/86h/1h/0h/0/0"),
+ amount=6_800,
+ prev_hash=TXHASH_df862e,
+ prev_index=0,
+ script_type=messages.InputScriptType.SPENDTAPROOT,
+ )
+ inp2 = messages.TxInputType(
+ # tb1p8tvmvsvhsee73rhym86wt435qrqm92psfsyhy6a3n5gw455znnpqm8wald
+ # m/86'/1'/0'/0/1 for "all all ... all" seed.
+ amount=13_000,
+ prev_hash=TXHASH_3ac32e,
+ prev_index=1,
+ script_pubkey=bytes.fromhex(
+ "51203ad9b641978673e88ee4d9f4e5d63400c1b2a8304c09726bb19d10ead2829cc2"
+ ),
+ script_type=messages.InputScriptType.EXTERNAL,
+ witness=b"",
+ script_sig=b"",
+ )
+ out1 = messages.TxOutputType(
+ address="tb1q7r9yvcdgcl6wmtta58yxf29a8kc96jkyxl7y88",
+ amount=15_000,
+ script_type=messages.OutputScriptType.PAYTOADDRESS,
+ )
+ out2 = messages.TxOutputType(
+ address_n=parse_path("m/86h/1h/0h/1/0"),
+ script_type=messages.OutputScriptType.PAYTOTAPROOT,
+ amount=6_800 + 13_000 - 200 - 15_000,
+ )
+
+ with pytest.raises(TrezorFailure, match="[Ee]xternal input"):
+ btc.sign_tx(
+ session, "Testnet", [inp1, inp2], [out1, out2], prev_txes=TX_CACHE_TESTNET
+ )
+
+
def test_p2wpkh_external_unverified(session: Session):
inp1 = messages.TxInputType(
# tb1qkvwu9g3k2pdxewfqr7syz89r3gj557l3uuf9r9
Why this scored 72/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.