fix(legacy): align coinjoin behavior with core
What changed, and why it matters
This commit tightens access controls in the older Trezor firmware (legacy) for a privacy feature called CoinJoin. Previously, after a host app was pre-authorized, any next message could consume that authorization. Now only specific message types (SignTx and GetOwnershipProof for preauthorization; GetAddress, GetPublicKey, and SignTx for an unlocked path) can use it. The vendor describes this as defense in depth, not a fix for a directly reachable attack, because the host that could trigger it already has related access. Still, without the change, a preauthorization could let GetAddress or GetOwnershipId touch the CoinJoin account without showing the user the usual confirmation screen.
Treat as a security hardening patch for the legacy firmware. Users relying on CoinJoin/SLIP-25 on legacy devices should update when a release containing this commit is available. Reviewers should verify that the allowlists match Core exactly and that no other message types legitimately rely on the broader authorization.
Security signals we found
Authorization scope restriction (allowlist)
Defense-in-depth hardening for SLIP-25/CoinJoin path
Behavioral alignment between legacy and Core firmware
Added regression test for forbidden key-path access via preauthorization
Evidence from the diff
The patch adds fsm_preMsgCleanup() in legacy/firmware/fsm.c, called from messages.c before each message is dispatched. It clears authorization_type if the incoming message is not SignTx or GetOwnershipProof, and clears unlock_path if the message is not GetAddress, GetPublicKey, or SignTx. This mirrors the behavior already present in Trezor Core. A test is added to verify that GetAddress cannot use a CoinJoin preauthorization to reach the SLIP-25 external chain without an explicit UnlockPath. The existing fsm_postMsgCleanup() already cleared authorization after most messages; the new function prevents consumption by non-allowlisted messages at dispatch time.
Changed components
legacy/firmware/fsm.clegacy/firmware/fsm.hlegacy/firmware/messages.ctests/device_tests/bitcoin/test_authorize_coinjoin.pyInspect captured patch +56 / −0
### legacy/firmware/.changelog.d/+coinjoin-preauthorization.changed
@@ -0,0 +1 @@
+Restrict which message types may use an authorization granted by DoPreauthorized or a path unlocked by UnlockPath.
### legacy/firmware/fsm.c
@@ -453,6 +453,32 @@ void fsm_abortWorkflows(void) {
#endif
}
+// Which messages may make use of an authorization granted by DoPreauthorized.
+static bool fsm_isPreauthorizedMessage(MessageType message_type) {
+ return message_type == MessageType_MessageType_SignTx ||
+ message_type == MessageType_MessageType_GetOwnershipProof;
+}
+
+// Which messages may make use of a path unlocked by UnlockPath.
+static bool fsm_isUnlockPathMessage(MessageType message_type) {
+ return message_type == MessageType_MessageType_GetAddress ||
+ message_type == MessageType_MessageType_GetPublicKey ||
+ message_type == MessageType_MessageType_SignTx;
+}
+
+void fsm_preMsgCleanup(MessageType message_type) {
+ // Drop the authorization and the unlocked path before processing a message
+ // which they do not apply to, so that it cannot gain access to the SLIP-25
+ // account without the user's confirmation.
+ if (!fsm_isPreauthorizedMessage(message_type)) {
+ authorization_type = 0;
+ }
+
+ if (!fsm_isUnlockPathMessage(message_type)) {
+ unlock_path = 0;
+ }
+}
+
void fsm_postMsgCleanup(MessageType message_type) {
if (message_type != MessageType_MessageType_DoPreauthorized) {
authorization_type = 0;
### legacy/firmware/fsm.h
@@ -163,6 +163,7 @@ bool fsm_getOwnershipId(uint8_t *script_pubkey, size_t script_pubkey_size,
uint8_t ownership_id[32]);
void fsm_abortWorkflows(void);
+void fsm_preMsgCleanup(MessageType message_type);
void fsm_postMsgCleanup(MessageType message_type);
#endif
### legacy/firmware/messages.c
@@ -61,6 +61,7 @@ void MessageProcessFunc(char type, char dir, uint16_t msg_id, void *ptr) {
const struct MessagesMap_t *m = MessagesMap;
while (m->type) {
if (type == m->type && dir == m->dir && msg_id == m->msg_id) {
+ fsm_preMsgCleanup(msg_id);
m->process_func(ptr);
fsm_postMsgCleanup(msg_id);
return;
### tests/device_tests/bitcoin/test_authorize_coinjoin.py
@@ -724,6 +724,33 @@ def test_cancel_authorization(session: Session):
)
+def test_preauthorized_message_type(session: Session):
+ # Ensure that a preauthorization cannot be used by a message type that it wasn't granted for.
+
+ 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,
+ )
+
+ # Only SignTx and GetOwnershipProof may use the preauthorization, so GetAddress must
+ # not gain access to the SLIP-25 account without going through UnlockPath.
+ session.call(messages.DoPreauthorized(), expect=messages.PreauthorizedRequest)
+
+ with pytest.raises(TrezorFailure, match="Forbidden key path"):
+ btc.get_address(
+ session,
+ "Testnet",
+ parse_path("m/10025h/1h/0h/1h/0/0"),
+ script_type=messages.InputScriptType.SPENDTAPROOT,
+ )
+
+
def test_get_public_key(session: Session):
ACCOUNT_PATH = parse_path("m/10025h/1h/0h/1h")
EXPECTED_XPUB = "tpubDEMKm4M3S2Grx5DHTfbX9et5HQb9KhdjDCkUYdH9gvVofvPTE6yb2MH52P9uc4mx6eFohUmfN1f4hhHNK28GaZnWRXr3b8KkfFcySo1SmXU"Why this scored 35/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.