Moderate 59 Bitcoin Committed May 16, 2026 · 07:23 UTC

sign: update output logic, rename and document output flags for clarity

cf79a3aa View source ↗

Public commit record

What the developer wrote

Authored by Mike Tolkachev

85/100 · Strong

sign: update output logic, rename and document output flags for clarity

Re-fixes change output marking so that it is only ever set if the
output is also marked as a wallet output. Downstream code can thus
check the change bit alone to determine wallet change.

✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode

The short version

What changed, and why it matters

This commit reworks how a Blockstream Jade hardware wallet labels transaction outputs. The key change is that the 'change' flag is now only set when the output is also confirmed as belonging to the wallet ('is ours'). Previously, downstream code had to check both 'validated' and 'change' bits to safely hide a change output; now it can rely on the 'change' bit alone. The commit also renames the flag from VALIDATED to IS_OURS and adds comments. The stated intent is to prevent a change output from being marked as change unless it is also confirmed as a wallet output, which could reduce the risk of an attacker tricking the device into hiding a payment to an external address.

Recommended action

Treat as a security-hardening fix with potential prior weakness. Review the previous commit that introduced the bug being re-fixed, and verify that all code paths setting OUTPUT_FLAG_CHANGE also set OUTPUT_FLAG_IS_OURS. Consider whether any downstream consumers still check the old two-bit combination and update them. No immediate emergency action is indicated, but a firmware release note should mention the fix.

Security signals we found

Change output flag now gated by wallet-ownership flag

UI hides change outputs based on single bit with runtime assertion

Green 2of3 outputs no longer marked as change because not marked as wallet-owned

Rename and documentation of output flags for clarity

Commit message explicitly says 'Re-fixes change output marking'

Technical analysis

Evidence from the diff

The patch refactors output flag semantics in the Jade signing code. OUTPUT_FLAG_VALIDATED is renamed to OUTPUT_FLAG_IS_OURS and documented. The logic in sign_psbt.c, sign_tx.c, sign_utils.c, and the UI is updated so that OUTPUT_FLAG_CHANGE is only set together with OUTPUT_FLAG_IS_OURS. In sign_tx.c, the Green 2of3 case no longer sets the change flag at all because the output is not marked as wallet-owned. The UI’s display_output() now hides outputs based solely on OUTPUT_FLAG_CHANGE, with an assertion that OUTPUT_FLAG_IS_OURS is also set. This is described as a re-fix for change-output marking.

Changed components

main/process/sign_psbt.cmain/process/sign_tx.cmain/process/sign_utils.cmain/ui.hmain/ui/sign_tx.c

Inspect captured patch +32 / −24

diff --git a/main/process/sign_psbt.c b/main/process/sign_psbt.c
index f81b160..65c8c47 100644
--- a/main/process/sign_psbt.c
+++ b/main/process/sign_psbt.c
@@ -531,16 +531,17 @@ static bool psbt_update_outputs(const network_t network_id, struct wally_psbt* p
                 // Fee output
                 JADE_ASSERT(!(outinfo->flags & OUTPUT_FLAG_CONFIDENTIAL));
                 JADE_ASSERT(outinfo->flags & OUTPUT_FLAG_HAS_UNBLINDED);
-                // Fee outputs can't be change, so may as well skip now
-                JADE_ASSERT(!(outinfo->flags & (OUTPUT_FLAG_VALIDATED | OUTPUT_FLAG_CHANGE)));
+                // Fee outputs can't be ours or change: skip further processing
+                JADE_ASSERT(!(outinfo->flags & (OUTPUT_FLAG_IS_OURS | OUTPUT_FLAG_CHANGE)));
                 continue;
             }
         }
 
         JADE_LOGD("Considering output %u for change", index);
 
-        // By default, assume not a validated or change output, and so user must verify
-        JADE_ASSERT(!(outinfo->flags & (OUTPUT_FLAG_VALIDATED | OUTPUT_FLAG_CHANGE)));
+        // Initially we assume the output isn't a wallet output or wallet
+        // change, so the user must explicitly confirm it.
+        JADE_ASSERT(!(outinfo->flags & (OUTPUT_FLAG_IS_OURS | OUTPUT_FLAG_CHANGE)));
 
         // Find the first key belonging to this signer
         if (!key_iter_output_begin_public(psbt, index, &iter)) {
@@ -596,7 +597,7 @@ static bool psbt_update_outputs(const network_t network_id, struct wally_psbt* p
             JADE_LOGI("Output %u singlesig %s path/script validated", index, is_change ? "change" : "receive");
 
             // Set appropriate flags
-            outinfo->flags |= OUTPUT_FLAG_VALIDATED;
+            outinfo->flags |= OUTPUT_FLAG_IS_OURS;
             if (is_change) {
                 outinfo->flags |= OUTPUT_FLAG_CHANGE;
             }
@@ -636,7 +637,7 @@ static bool psbt_update_outputs(const network_t network_id, struct wally_psbt* p
             JADE_LOGI("Output %u green-multisig path/script validated", index);
 
             // Set appropriate flags - note Green wallet-output is always assumed to be change
-            outinfo->flags |= (OUTPUT_FLAG_VALIDATED | OUTPUT_FLAG_CHANGE);
+            outinfo->flags |= (OUTPUT_FLAG_IS_OURS | OUTPUT_FLAG_CHANGE);
 
         } else if (signing_flags == (PSBT_SIGNING_MULTISIG | PSBT_SIGNING_SINGLE_MULTISIG_RECORD)) {
             // Generic multisig or descriptor
@@ -666,7 +667,7 @@ static bool psbt_update_outputs(const network_t network_id, struct wally_psbt* p
                 wallet_name);
 
             // Set appropriate flags
-            outinfo->flags |= OUTPUT_FLAG_VALIDATED;
+            outinfo->flags |= OUTPUT_FLAG_IS_OURS;
             if (is_change) {
                 outinfo->flags |= OUTPUT_FLAG_CHANGE;
             }
diff --git a/main/process/sign_tx.c b/main/process/sign_tx.c
index 0b8d64b..81aa234 100644
--- a/main/process/sign_tx.c
+++ b/main/process/sign_tx.c
@@ -124,8 +124,9 @@ static bool params_signing_outputs(jade_process_t* process, const CborValue* par
 
         JADE_ASSERT(!cbor_value_at_end(&arrayItem));
 
-        // By default, assume not a validated or change output, and so user must verify
-        JADE_ASSERT(!(outinfo->flags & (OUTPUT_FLAG_VALIDATED | OUTPUT_FLAG_CHANGE)));
+        // Initially we assume the output isn't a wallet output or wallet
+        // change, so the user must explicitly confirm it.
+        JADE_ASSERT(!(outinfo->flags & (OUTPUT_FLAG_IS_OURS | OUTPUT_FLAG_CHANGE)));
         if (cbor_value_is_map(&arrayItem)) {
             // Output path info passed, try to verify output
             JADE_LOGD("Output %u has output/change data passed", i);
@@ -308,13 +309,13 @@ static bool params_signing_outputs(jade_process_t* process, const CborValue* par
             // Set appropriate flags
             if (!is_green_2of3) {
                 // Note for Green 2of3 we don't trust the host-provided xpub, so
-                // we do not mark this output as a validated wallet output.
+                // we do not mark this output as belonging to our wallet.
                 // TODO: Allow registration of 2of3 accounts so the user
                 //       doesn't have to confirm legitimate wallet outputs.
-                outinfo->flags |= OUTPUT_FLAG_VALIDATED;
-            }
-            if (is_change) {
-                outinfo->flags |= OUTPUT_FLAG_CHANGE;
+                outinfo->flags |= OUTPUT_FLAG_IS_OURS;
+                if (is_change) {
+                    outinfo->flags |= OUTPUT_FLAG_CHANGE;
+                }
             }
         }
         const CborError err = cbor_value_advance(&arrayItem);
diff --git a/main/process/sign_utils.c b/main/process/sign_utils.c
index 7de2dd0..d3004c0 100644
--- a/main/process/sign_utils.c
+++ b/main/process/sign_utils.c
@@ -577,7 +577,7 @@ bool update_elements_outputs(
             outinfo->flags |= OUTPUT_FLAG_CONFIDENTIAL;
 
             // NOTE: This is not valid if this output has been validated as belonging to this wallet
-            if (outinfo->flags & OUTPUT_FLAG_VALIDATED) {
+            if (outinfo->flags & OUTPUT_FLAG_IS_OURS) {
                 *errmsg = "Missing blinding information for wallet output";
                 return false;
             }
@@ -625,7 +625,7 @@ bool validate_elements_outputs(const network_t network_id, const struct wally_tx
 
         // If the output has been verified as belonging to this wallet, we can
         // use it to validate some part of any passed input- or output- summary.
-        if (outinfo->flags & OUTPUT_FLAG_VALIDATED) {
+        if (outinfo->flags & OUTPUT_FLAG_IS_OURS) {
             JADE_ASSERT(outinfo->flags & OUTPUT_FLAG_HAS_UNBLINDED);
 
             if (outinfo->flags & OUTPUT_FLAG_CHANGE) {
diff --git a/main/ui.h b/main/ui.h
index 85770d1..5dfd72a 100644
--- a/main/ui.h
+++ b/main/ui.h
@@ -84,11 +84,16 @@ typedef struct {
 // Whether QR Frame Guides (box corners) should be shown
 typedef enum { QR_GUIDE_HIDE, QR_GUIDE_SHOW } qr_guide_type_t;
 
-#define OUTPUT_FLAG_CONFIDENTIAL 1
-#define OUTPUT_FLAG_HAS_BLINDING_KEY 2
-#define OUTPUT_FLAG_VALIDATED 4
-#define OUTPUT_FLAG_CHANGE 8
-#define OUTPUT_FLAG_HAS_UNBLINDED 16
+// Output is confidential/blinded
+#define OUTPUT_FLAG_CONFIDENTIAL (1 << 0)
+// Output has a blinding public key
+#define OUTPUT_FLAG_HAS_BLINDING_KEY (1 << 1)
+// Output is ours (belongs to a wallet controlled by this Jade)
+#define OUTPUT_FLAG_IS_OURS (1 << 2)
+// Output is a change output for this wallet (only set with OUTPUT_FLAG_IS_OURS)
+#define OUTPUT_FLAG_CHANGE (1 << 3)
+// Output has unblinded asset and value
+#define OUTPUT_FLAG_HAS_UNBLINDED (1 << 4)
 
 // Progress bar
 typedef struct {
diff --git a/main/ui/sign_tx.c b/main/ui/sign_tx.c
index 8e02dd6..3b5833c 100644
--- a/main/ui/sign_tx.c
+++ b/main/ui/sign_tx.c
@@ -53,8 +53,9 @@ static bool display_output(
             return true;
         }
 
-        if (output_info[i].flags & OUTPUT_FLAG_VALIDATED && output_info[i].flags & OUTPUT_FLAG_CHANGE) {
+        if (output_info[i].flags & OUTPUT_FLAG_CHANGE) {
             // Hide change outputs which have already been internally validated
+            JADE_ASSERT(output_info[i].flags & OUTPUT_FLAG_IS_OURS);
             return false;
         }
     }
@@ -441,7 +442,7 @@ bool show_btc_transaction_outputs_activity(
         // Free all existing activities between outputs
         gui_set_current_activity_ex(act_clear, true);
 
-        const bool is_wallet_output = output_info && (output_info[i].flags & OUTPUT_FLAG_VALIDATED);
+        const bool is_wallet_output = output_info && (output_info[i].flags & OUTPUT_FLAG_IS_OURS);
 
         char title[16];
         int ret = snprintf(title, sizeof(title), "Output %ld/%ld", nDisplayedOutput, nTotalOutputsDisplayed);
@@ -503,7 +504,7 @@ bool show_elements_transaction_outputs_activity(const network_t network_id, cons
         // Free all existing activities between outputs
         gui_set_current_activity_ex(act_clear, true);
 
-        const bool is_wallet_output = output_info[i].flags & OUTPUT_FLAG_VALIDATED;
+        const bool is_wallet_output = output_info[i].flags & OUTPUT_FLAG_IS_OURS;
 
         char title[16];
         const int ret = snprintf(title, sizeof(title), "Output %ld/%ld", nDisplayedOutput, nTotalOutputsDisplayed);

Risk score

Why this scored 59/100

Our methodology →

Potential impact 18/30

Exploitability 12/25

Stealth signal 10/15

Affected reach 10/15

Confidence 6/10

Evidence quality 3/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 59/100

Community notes

Related watches

Liquid (taproot): add sign_liquid_tx genesis blockhash support

rpc: rename get_boolean to get_bool

rpc: refactor app code to use getters with default values