sign: match sign_tx logic for output commitment checking
What the developer wrote
Authored by Jon Griffiths
sign: match sign_tx logic for output commitment checking
What changed, and why it matters
This commit fixes a logic gap in how Blockstream Jade checks cryptographic commitments for transaction outputs during signing. Previously, the code treated a missing commitment the same as an invalid one, which could cause the device to reject valid transactions or accept improperly validated ones. The change aligns this code path with the existing sign_tx logic, making output commitment handling consistent and more secure.
Review related signing paths for similar commitment-handling inconsistencies; verify that params_commitment_data() callers elsewhere correctly distinguish missing, valid, and invalid commitments; consider regression tests covering all three return cases.
Security signals we found
Inconsistent commitment validation between signing code paths
Possible transaction signing failure or acceptance of unvalidated output commitments
Logic alignment with existing sign_tx commitment handling
Evidence from the diff
In main/process/sign_utils.c, params_trusted_commitments() now distinguishes three cases when calling params_commitment_data(): (1) valid commitments returned true with no error, (2) invalid commitments returned false with an error message, and (3) no commitments present returned false with no error. Previously it only checked whether errmsg was set, conflating cases 2 and 3. The fix adds an explicit success branch and only jumps to cleanup on actual validation failure, matching the logic already used in sign_tx.
Changed components
main/process/sign_utils.cparams_trusted_commitments()params_commitment_data()Transaction output commitment validationInspect captured patch +5 / −2
diff --git a/main/process/sign_utils.c b/main/process/sign_utils.c
index ef4fbdf..25ee20e 100644
--- a/main/process/sign_utils.c
+++ b/main/process/sign_utils.c
@@ -516,8 +516,11 @@ bool params_trusted_commitments(
}
// Populate commitments data for the tx output if present
- params_commitment_data(&arrayItem, &commitments[i], &tx->outputs[i], &errmsg);
- if (errmsg) {
+ if (params_commitment_data(&arrayItem, &commitments[i], &tx->outputs[i], &errmsg)) {
+ // Valid output commitments
+ JADE_ASSERT(!errmsg);
+ } else if (errmsg) {
+ // Invalid input commitments (rather than simply not present)
goto cleanup;
}
Why this scored 57/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.