tx: fix check for short commitments
What the developer wrote
Authored by Jon Griffiths
tx: fix check for short commitments
Thanks to @erickcestari for reporting and suggesting a fix.
What changed, and why it matters
This commit fixes a bounds-check bug when reading confidential asset or value commitments in Bitcoin/Elements transactions. Before the fix, the code read the first byte of a commitment to decide its type without first confirming that at least one byte remained in the buffer. A malformed, truncated transaction could therefore read one byte past the end of the input data, potentially causing an out-of-bounds read. The patch adds a one-byte length check before that first read.
Review whether this parsing path is reachable from untrusted network input or user-supplied PSBT/transaction data. If so, treat as a security fix and release an advisory or patch note. Add regression tests with truncated commitments to prevent reintroduction.
Security signals we found
Out-of-bounds read in transaction deserialization
Missing bounds check before reading commitment prefix byte
Confidential transaction / asset commitment parsing
Fix credited to external reporter @erickcestari
Evidence from the diff
In src/transaction.c, the ensure_commitment() macro inspects *dst to branch on commitment prefix types. The original implementation dereferenced dst before calling ensure_n(sizeof(uint8_t)). The patch adds ensure_n(sizeof(uint8_t)) before the switch so that analyze_tx() fails cleanly when the buffer is exhausted, rather than performing an out-of-bounds read. The surrounding code already had similar checks inside each branch; this change closes the gap for the prefix byte itself. The bug is in transaction parsing, a reachable code path for any caller that deserializes raw transactions.
Changed components
src/transaction.canalyze_tx()ensure_commitment() macroConfidential asset/value commitment parsingInspect captured patch +1 / −0
diff --git a/src/transaction.c b/src/transaction.c
index 4d191f4..f072547 100644
--- a/src/transaction.c
+++ b/src/transaction.c
@@ -2243,6 +2243,7 @@ static int analyze_tx(const unsigned char *bytes, size_t bytes_len,
ensure_n(*dst)
#define ensure_commitment(dst, explicit_siz, prefix_a, prefix_b) \
+ ensure_n(sizeof(uint8_t)); \
switch (*dst) { \
case WALLY_TX_ASSET_CT_EMPTY_PREFIX: \
ensure_n(sizeof(uint8_t)); \
Why this scored 60/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.