What the developer wrote
Authored by Erick Cestari
wire: bound witness read to remaining slab
Add a bounds check against the remaining slab capacity so oversized
witness items return a decode error. Add a regression test for witness
items that overflow the decode slab.
What changed, and why it matters
This commit fixes a bug in btcd's transaction decoding where a malformed Bitcoin transaction could claim witness data larger than the internal decode buffer. Before the fix, this could cause a panic (crash) when the code tried to copy data into a too-small buffer. The patch adds a size check so the transaction is rejected cleanly instead of crashing the node.
Apply the patch. Nodes running affected versions should upgrade to avoid a remote DoS via malformed witness transactions. Monitor for attempts to send crafted witness transactions with oversized item claims.
Security signals we found
Out-of-bounds slice access/panic in network message decoder
Malformed P2P transaction can trigger crash (denial of service)
Missing bounds check on remaining buffer capacity
Regression test explicitly named 'TestTxWitnessOverflowPanic'
Evidence from the diff
In wire/msgtx.go, readScriptBuf decodes variable-length witness scripts into a reusable slab buffer s. The existing check only verified count <= maxWitnessItemSize (4 MiB), but did not verify that count fits in the remaining bytes of the actual slab. A malicious or malformed witness item could pass the max-item check while exceeding the remaining slab capacity, leading to an out-of-bounds slice access/panic. The patch adds if count > uint64(len(s)) and returns a MessageError. A regression test constructs a witness transaction whose first item consumes most of the slab and whose second item’s claimed length overflows the remainder, verifying decode returns an error rather than panicking.
Changed components
wire/msgtx.go:readScriptBufwire.MsgTx.BtcDecode with WitnessEncodingbtcd P2P transaction decodingInspect captured patch +76 / −1
diff --git a/wire/msgtx.go b/wire/msgtx.go
index e4f3051..4afe33b 100644
--- a/wire/msgtx.go
+++ b/wire/msgtx.go
@@ -1034,7 +1034,17 @@ func readScriptBuf(r io.Reader, pver uint32, buf, s []byte,
// upper bound on this count.
if count > maxWitnessItemSize {
str := fmt.Sprintf("%s is larger than the max allowed size "+
- "[count %d, max %d]", fieldName, count, maxWitnessItemSize)
+ "[count %d, max %d]", fieldName, count,
+ maxWitnessItemSize)
+ return nil, messageError("readScript", str)
+ }
+
+ // Ensure the claimed script length fits in the remaining
+ // decode slab.
+ if count > uint64(len(s)) {
+ str := fmt.Sprintf("%s exceeds remaining buffer "+
+ "capacity [count %d, remaining %d]",
+ fieldName, count, len(s))
return nil, messageError("readScript", str)
}
diff --git a/wire/msgtx_test.go b/wire/msgtx_test.go
index 62d5709..ccde070 100644
--- a/wire/msgtx_test.go
+++ b/wire/msgtx_test.go
@@ -1166,3 +1166,68 @@ var multiWitnessTxEncodedNonZeroFlag = []byte{
// multiTxPkScriptLocs is the location information for the public key scripts
// located in multiWitnessTx.
var multiWitnessTxPkScriptLocs = []int{58}
+
+// TestTxWitnessOverflowPanic ensures that decoding a witness tx where
+// cumulative witness item lengths exceed the script slab capacity
+// returns a decode error instead of panicking on an out-of-bounds
+// slice.
+func TestTxWitnessOverflowPanic(t *testing.T) {
+ // Build a minimal witness tx with one input, zero outputs,
+ // and two witness items whose combined claimed lengths
+ // exceed the scriptSlabSize (4 MiB) decode buffer.
+ //
+ // Item 1: 3 000 000 bytes (fits in slab, < maxWitnessItemSize)
+ // Item 2: 2 000 000 bytes (passes maxWitnessItemSize but
+ // overflows remaining slab capacity of ~1.19 MiB)
+ const (
+ firstLen = 3_000_000
+ secondLen = 2_000_000
+ )
+
+ var buf bytes.Buffer
+
+ // tx version = 2
+ buf.Write([]byte{0x02, 0x00, 0x00, 0x00})
+
+ // Segwit marker + flag
+ buf.WriteByte(TxFlagMarker)
+ buf.WriteByte(byte(WitnessFlag))
+
+ // 1 input
+ WriteVarInt(&buf, 0, 1)
+
+ // Previous outpoint: 32-byte zero hash + index 0
+ buf.Write(make([]byte, 32))
+ buf.Write([]byte{0x00, 0x00, 0x00, 0x00})
+
+ // Empty signature script
+ WriteVarInt(&buf, 0, 0)
+
+ // Sequence
+ buf.Write([]byte{0x00, 0x00, 0x00, 0x00})
+
+ // 0 outputs
+ WriteVarInt(&buf, 0, 0)
+
+ // Witness: 2 stack items for the single input
+ WriteVarInt(&buf, 0, 2)
+
+ // First witness item: firstLen bytes of zeros.
+ WriteVarInt(&buf, 0, firstLen)
+ buf.Write(make([]byte, firstLen))
+
+ // Second witness item: only write the varint claiming
+ // secondLen bytes. The actual data is irrelevant because
+ // the bounds check must reject before reading it.
+ WriteVarInt(&buf, 0, secondLen)
+
+ // No locktime needed; decoding should fail before that.
+
+ var msg MsgTx
+ r := bytes.NewReader(buf.Bytes())
+ err := msg.BtcDecode(r, ProtocolVersion, WitnessEncoding)
+ require.Error(t, err)
+
+ var msgErr *MessageError
+ require.ErrorAs(t, err, &msgErr)
+}
Why this scored 72/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.