fix(legacy): abort on coinjoin coin-name mismatch
What the developer wrote
Authored by Andrew Kozlik
fix(legacy): abort on coinjoin coin-name mismatch
Enforces a check aligning the behavior with core. The missing check did
not yield anything an attacker couldn't get from a legitimate coinjoin
round on the authorized coin. The inputs signed are always from the
authorized SLIP-25 account regardless of what coin_name claims.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What changed, and why it matters
This commit fixes a bug in the older Trezor firmware (legacy) where a CoinJoin signing session would not be properly aborted if the requested coin name did not match the one in the prior authorization. Before the fix, the device sent an error message but continued processing, which could allow a malicious or mismatched transaction to proceed. The fix adds an explicit abort and return so the signing stops immediately, matching the behavior of newer Trezor firmware (core). The commit message downplays the practical risk, noting an attacker could not gain more than from a legitimate CoinJoin on the authorized coin.
Treat as a low-to-moderate security hardening fix. Merge the patch and ensure the new regression test passes. Review other failure paths in legacy signing.c for similar missing abort/return patterns after fsm_sendFailure().
Security signals we found
Missing control-flow termination after security failure (CWE-691, CWE-248)
CoinJoin authorization bypass risk due to incomplete abort on coin-name mismatch
Behavior alignment with core firmware (defense-in-depth consistency)
New regression test added for wrong coin name in preauthorized CoinJoin
Evidence from the diff
In legacy/firmware/signing.c, init_coinjoin() previously called fsm_sendFailure() when coin->coin_name differed from authorization->coin_name, but did not call signing_abort() or return false. Execution would fall through to memcpy(&coinjoin_authorization, authorization, …), potentially leaving signing state active. The patch adds signing_abort(); return false; immediately after the failure send. A new test, test_wrong_coin_name, verifies that signing a Bitcoin transaction with a Testnet authorization fails with ‘Unauthorized operation’ on legacy and ‘Forbidden key path’ on core.
Changed components
legacy/firmware/signing.clegacy CoinJoin signing flowtests/device_tests/bitcoin/test_authorize_coinjoin.pyInspect captured patch +75 / −0
### legacy/firmware/.changelog.d/+coinjoin-coin-name.fixed
@@ -0,0 +1 @@
+Abort coinjoin signing if the coin name in SignTx does not match the coinjoin authorization.
### legacy/firmware/signing.c
@@ -1387,6 +1387,8 @@ static bool init_coinjoin(const SignTx *msg,
if (strcmp(coin->coin_name, authorization->coin_name) != 0) {
fsm_sendFailure(FailureType_Failure_ProcessError,
_("Unauthorized operation."));
+ signing_abort();
+ return false;
}
memcpy(&coinjoin_authorization, authorization,
### tests/device_tests/bitcoin/test_authorize_coinjoin.py
@@ -624,6 +624,78 @@ def test_wrong_account_type(session: Session):
)
+def test_wrong_coin_name(session: Session):
+ # Ensure that an authorization granted for one coin cannot be used to sign a
+ # coinjoin transaction on another coin.
+
+ inputs = [
+ messages.TxInputType(
+ address_n=parse_path("m/10025h/1h/0h/1h/1/0"),
+ amount=7_289_000,
+ prev_hash=FAKE_TXHASH_f982c0,
+ prev_index=1,
+ script_type=messages.InputScriptType.SPENDTAPROOT,
+ ),
+ ]
+
+ input_script_pubkeys = [
+ bytes.fromhex(
+ "51202f436892d90fb2665519efa3d9f0f5182859124f179486862c2cd7a78ea9ac19"
+ ),
+ ]
+
+ outputs = [
+ # Our change output.
+ messages.TxOutputType(
+ # tb1pchruvduckkwuzm5hmytqz85emften5dnmkqu9uhfxwfywaqhuu0qjggqyp
+ address_n=parse_path("m/10025h/1h/0h/1h/1/2"),
+ amount=7_289_000 - 490,
+ script_type=messages.OutputScriptType.PAYTOTAPROOT,
+ ),
+ ]
+
+ output_script_pubkeys = [
+ bytes.fromhex(
+ "5120c5c7c63798b59dc16e97d916011e99da5799d1b3dd81c2f2e93392477417e71e"
+ ),
+ ]
+
+ coinjoin_req = make_coinjoin_request(
+ "www.example.com",
+ inputs,
+ input_script_pubkeys,
+ outputs,
+ output_script_pubkeys,
+ no_fee_indices=[],
+ )
+
+ btc.authorize_coinjoin(
+ session,
+ coordinator="www.example.com",
+ max_rounds=10,
+ max_coordinator_fee_rate=500_000, # 0.5 %
+ max_fee_per_kvbyte=3500,
+ n=parse_path("m/10025h/1h/0h/1h"),
+ coin_name="Testnet",
+ script_type=messages.InputScriptType.SPENDTAPROOT,
+ )
+
+ # Legacy rejects the coin name up front in SignTx. Core never unlocks the Testnet
+ # SLIP-25 path for a Bitcoin keychain, so it rejects the input path instead.
+ expected = "Forbidden key path" if is_core(session) else "Unauthorized operation"
+
+ with pytest.raises(TrezorFailure, match=expected):
+ btc.sign_tx(
+ session,
+ "Bitcoin",
+ inputs,
+ outputs,
+ prev_txes=TX_CACHE_MAINNET,
+ coinjoin_request=coinjoin_req,
+ preauthorized=True,
+ )
+
+
def test_cancel_authorization(session: Session):
# Ensure that a preauthorized GetOwnershipProof fails if the commitment_data doesn't match the coordinator.
Why this scored 34/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.