sign_psbt: use the psbt tx directly if possible
What the developer wrote
Authored by Jon Griffiths
sign_psbt: use the psbt tx directly if possible
Since we aren't finalizing the tx, we can use it directly for v0 PSBTs
and avoid creating a new one.
What changed, and why it matters
This commit changes how a Bitcoin hardware wallet (Blockstream Jade) obtains the transaction it signs from a Partially Signed Bitcoin Transaction (PSBT). For older v0 PSBTs, it now uses the transaction embedded in the PSBT directly instead of extracting a fresh copy. The main risk is that the code now trusts the PSBT's internal transaction more directly, but the commit also adds a safety check that the transaction exists and matches the PSBT's input/output counts. There is no clear security bug visible in the diff, but the change touches sensitive signing logic and is described only as a simplification, not a security fix.
Review whether psbt->tx is validated elsewhere before sign_psbt is called and confirm that skipping wally_tx_free for v0 does not leave a dangling or double-free risk if the PSBT is later mutated or freed. Consider adding regression tests for v0 and v2 PSBT signing paths and verify that the direct tx pointer cannot differ from an extracted tx in a way that affects signature hashes.
Security signals we found
Change in transaction derivation path for PSBT signing
Pointer reuse of psbt->tx instead of extracted copy for v0 PSBTs
Ownership/lifetime change: wally_tx_free skipped for v0
Assertion broadened to check tx non-null after conditional assignment
No explicit security rationale or bug reference in commit message
Evidence from the diff
In main/process/sign_psbt.c, the signing flow previously called wally_psbt_extract() with WALLY_PSBT_EXTRACT_NON_FINAL to obtain a wally_tx for all PSBT versions. The patch short-circuits this for WALLY_PSBT_VERSION_0 by assigning psbt->tx directly and skips wally_tx_free() for v0 because the tx is owned by the PSBT. It also moves the non-null assertion to cover both paths. The change reduces allocation/copying but means the signer now operates on the exact tx pointer stored in the v0 PSBT. The diff does not show validation of the PSBT tx beyond count checks, and it does not show whether psbt->tx is otherwise trusted. No explicit security relevance, CVE, or researcher attribution is present in the commit.
Changed components
main/process/sign_psbt.cPSBT signing path for v0 PSBTswally_tx lifetime management in sign_psbtInspect captured patch +9 / −5
diff --git a/main/process/sign_psbt.c b/main/process/sign_psbt.c
index eb94bf4..e3d44ac 100644
--- a/main/process/sign_psbt.c
+++ b/main/process/sign_psbt.c
@@ -682,13 +682,15 @@ int sign_psbt(const network_t network_id, struct wally_psbt* psbt, const char**
return CBOR_RPC_BAD_PARAMETERS;
}
- // Txn data must be available
- struct wally_tx* tx = NULL;
- if (wally_psbt_extract(psbt, WALLY_PSBT_EXTRACT_NON_FINAL, &tx) != WALLY_OK || !tx) {
+ struct wally_tx* tx = NULL; // Holds the extracted tx
+ // Fetch the tx to sign
+ if (psbt->version == WALLY_PSBT_VERSION_0) {
+ tx = psbt->tx; // For v0, use the PSBT tx directly
+ } else if (wally_psbt_extract(psbt, WALLY_PSBT_EXTRACT_NON_FINAL, &tx) != WALLY_OK) {
*errmsg = "Failed to extract valid txn from passed psbt";
return CBOR_RPC_BAD_PARAMETERS;
}
- JADE_ASSERT(tx->num_inputs == psbt->num_inputs && tx->num_outputs == psbt->num_outputs);
+ JADE_ASSERT(tx && tx->num_inputs == psbt->num_inputs && tx->num_outputs == psbt->num_outputs);
key_iter iter; // Holds any public/private key in use
SENSITIVE_PUSH(&iter, sizeof(iter));
@@ -982,7 +984,9 @@ int sign_psbt(const network_t network_id, struct wally_psbt* psbt, const char**
cleanup:
SENSITIVE_POP(&iter);
- JADE_WALLY_VERIFY(wally_tx_free(tx));
+ if (tx && psbt->version != WALLY_PSBT_VERSION_0) {
+ JADE_WALLY_VERIFY(wally_tx_free(tx));
+ }
free(descriptor);
free(multisig_data);
free(signing_inputs);
Why this scored 24/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.