psbt: don’t accumlate 0 lenth widtness data
What the developer wrote
Authored by Dusty Daemon
psbt: don’t accumlate 0 lenth widtness data
What changed, and why it matters
This commit fixes a small bug in how Core Lightning builds Bitcoin witness data from a PSBT. Previously, when a witness item had a declared length of zero, the code would still try to copy data from a pointer that could be NULL. The fix skips the copy when the length is zero and adds an assertion that the pointer is valid when length is non-zero. This is a defensive correctness fix; it likely prevents a potential NULL-pointer read or undefined behavior rather than an obvious remote exploit.
Treat as a low-risk bug fix. Review whether zero-length witness items can be injected via external PSBT input and confirm the fix is included in the next release. No urgent response appears necessary absent evidence of a reproducible crash or exploit.
Security signals we found
NULL pointer handling
defensive assertion added
memory/serialization correctness
witness data parsing
Evidence from the diff
In common/psbt_internal.c, psbt_to_witnesses() serializes witness stack items by writing a varint length followed by the item bytes. Before this patch, tal_expand() was called unconditionally with wtx_s->items[j].witness even when witness_len was 0. With a zero-length item the witness pointer may be NULL, so tal_expand on a NULL pointer with length 0 is at best a no-op and at worst implementation-defined/undefined. The patch guards the copy with if (witness_len) and asserts witness != NULL for non-zero lengths. This is a robustness/correctness improvement in PSBT serialization.
Changed components
common/psbt_internal.cpsbt_to_witnesses()PSBT to transaction witness serializationInspect captured patch +6 / −2
diff --git a/common/psbt_internal.c b/common/psbt_internal.c
index 4b0c98e..44501d6 100644
--- a/common/psbt_internal.c
+++ b/common/psbt_internal.c
@@ -154,8 +154,12 @@ psbt_to_witnesses(const tal_t *ctx,
add_varint(&wit->witness_data, wtx_s->num_items);
for (size_t j = 0; j < wtx_s->num_items; j++) {
add_varint(&wit->witness_data, wtx_s->items[j].witness_len);
- tal_expand(&wit->witness_data, wtx_s->items[j].witness,
- wtx_s->items[j].witness_len);
+ if (wtx_s->items[j].witness_len) {
+ assert(wtx_s->items[j].witness);
+ tal_expand(&wit->witness_data,
+ wtx_s->items[j].witness,
+ wtx_s->items[j].witness_len);
+ }
}
tal_arr_expand(&witnesses, wit);
Why this scored 32/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.