mempool: make txn's below 65 non-witness bytes non-standard

This is to mitigate CVE-2017-12842. Along the way, also error when
deserializing transactions that have the witness marker flag set
but have no witnesses. This matches Bitcoin Core's behaviour initially
introduced here bitcoin/bitcoin#14039. Allowing
such transactions is benign, but this makes sure that our parsing code
matches Core's exactly.

mempool/mempool.go

@@ -49,6 +49,10 @@ const (
 	// can be evicted from the mempool when accepting a transaction
 	// replacement.
 	MaxReplacementEvictions = 100
+
+	// Transactions smaller than 65 non-witness bytes are not relayed to
+	// mitigate CVE-2017-12842.
+	MinStandardTxNonWitnessSize = 65
 )
 
 // Tag represents an identifier to use for tagging orphan transactions.  The
@@ -1355,6 +1359,12 @@ func (mp *TxPool) checkMempoolAcceptance(tx *btcutil.Tx,
 		return nil, txRuleError(wire.RejectDuplicate, str)
 	}
 
+	// Disallow transactions under the minimum standardness size.
+	if tx.MsgTx().SerializeSizeStripped() < MinStandardTxNonWitnessSize {
+		str := fmt.Sprintf("tx %v is too small", txHash)
+		return nil, txRuleError(wire.RejectNonstandard, str)
+	}
+
 	// Perform preliminary sanity checks on the transaction. This makes use
 	// of blockchain which contains the invariant rules for what
 	// transactions are allowed into blocks.

wire/msgtx.go

@@ -111,6 +111,14 @@ const (
 	maxWitnessItemSize = 4_000_000
 )
 
+var (
+	// errSuperfluousWitnessRecord is returned during tx deserialization when
+	// a tx has the witness marker flag set but has no witnesses.
+	errSuperfluousWitnessRecord = fmt.Errorf(
+		"witness flag set but tx has no witnesses",
+	)
+)
+
 // TxFlagMarker is the first byte of the FLAG field in a bitcoin tx
 // message. It allows decoders to distinguish a regular serialized
 // transaction from one that would require a different parsing logic.
@@ -579,8 +587,7 @@ func (msg *MsgTx) btcDecode(r io.Reader, pver uint32, enc MessageEncoding,
 			txin.Witness = make([][]byte, witCount)
 			for j := uint64(0); j < witCount; j++ {
 				txin.Witness[j], err = readScriptBuf(
-					r, pver, buf, sbuf, maxWitnessItemSize,
-					"script witness item",
+					r, pver, buf, sbuf, "script witness item",
 				)
 				if err != nil {
 					return err
@@ -589,6 +596,12 @@ func (msg *MsgTx) btcDecode(r io.Reader, pver uint32, enc MessageEncoding,
 				sbuf = sbuf[len(txin.Witness[j]):]
 			}
 		}
+
+		// Check that if the witness flag is set that we actually have
+		// witnesses. This check is also done by bitcoind.
+		if !msg.HasWitness() {
+			return errSuperfluousWitnessRecord
+		}
 	}
 
 	if _, err := io.ReadFull(r, buf[:4]); err != nil {
@@ -982,7 +995,7 @@ func writeOutPointBuf(w io.Writer, pver uint32, version int32, op *OutPoint,
 //
 // NOTE: b MUST either be nil or at least an 8-byte slice.
 func readScriptBuf(r io.Reader, pver uint32, buf, s []byte,
-	maxAllowed uint32, fieldName string) ([]byte, error) {
+	fieldName string) ([]byte, error) {
 
 	count, err := ReadVarIntBuf(r, pver, buf)
 	if err != nil {
@@ -992,9 +1005,9 @@ func readScriptBuf(r io.Reader, pver uint32, buf, s []byte,
 	// Prevent byte array larger than the max message size.  It would
 	// be possible to cause memory exhaustion and panics without a sane
 	// upper bound on this count.
-	if count > uint64(maxAllowed) {
+	if count > maxWitnessItemSize {
 		str := fmt.Sprintf("%s is larger than the max allowed size "+
-			"[count %d, max %d]", fieldName, count, maxAllowed)
+			"[count %d, max %d]", fieldName, count, maxWitnessItemSize)
 		return nil, messageError("readScript", str)
 	}
 
@@ -1021,8 +1034,9 @@ func readTxInBuf(r io.Reader, pver uint32, version int32, ti *TxIn,
 		return err
 	}
 
-	ti.SignatureScript, err = readScriptBuf(r, pver, buf, s, MaxMessagePayload,
-		"transaction input signature script")
+	ti.SignatureScript, err = readScriptBuf(
+		r, pver, buf, s, "transaction input signature script",
+	)
 	if err != nil {
 		return err
 	}
@@ -1085,8 +1099,7 @@ func readTxOutBuf(r io.Reader, pver uint32, version int32, to *TxOut,
 	to.Value = int64(littleEndian.Uint64(buf))
 
 	to.PkScript, err = readScriptBuf(
-		r, pver, buf, s, MaxMessagePayload,
-		"transaction output public key script",
+		r, pver, buf, s, "transaction output public key script",
 	)
 	return err
 }

wire/msgtx_test.go

@@ -6,6 +6,7 @@ package wire
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"reflect"
@@ -849,6 +850,17 @@ func TestTxOutPointFromString(t *testing.T) {
 	}
 }
 
+// TestTxSuperfluousWitnessRecord ensures that btcd fails to parse a tx with
+// the witness marker flag set but without any actual witnesses.
+func TestTxSuperfluousWitnessRecord(t *testing.T) {
+	m := &MsgTx{}
+	rbuf := bytes.NewReader(multiWitnessFlagNoWitness)
+	err := m.BtcDecode(rbuf, ProtocolVersion, WitnessEncoding)
+	if !errors.Is(err, errSuperfluousWitnessRecord) {
+		t.Fatalf("should have failed with %v", errSuperfluousWitnessRecord)
+	}
+}
+
 // multiTx is a MsgTx with an input and output and used in various tests.
 var multiTx = &MsgTx{
 	Version: 1,
@@ -1001,6 +1013,33 @@ var multiWitnessTx = &MsgTx{
 	},
 }
 
+// multiWitnessFlagNoWitness is the wire encoded bytes for multiWitnessTx with
+// the witness flag set but with witnesses omitted.
+var multiWitnessFlagNoWitness = []byte{
+	0x1, 0x0, 0x0, 0x0, // Version
+	TxFlagMarker, // Marker byte indicating 0 inputs, or a segwit encoded tx
+	WitnessFlag,  // Flag byte
+	0x1,          // Varint for number of inputs
+	0xa5, 0x33, 0x52, 0xd5, 0x13, 0x57, 0x66, 0xf0,
+	0x30, 0x76, 0x59, 0x74, 0x18, 0x26, 0x3d, 0xa2,
+	0xd9, 0xc9, 0x58, 0x31, 0x59, 0x68, 0xfe, 0xa8,
+	0x23, 0x52, 0x94, 0x67, 0x48, 0x1f, 0xf9, 0xcd, // Previous output hash
+	0x13, 0x0, 0x0, 0x0, // Little endian previous output index
+	0x0,                    // No sig script (this is a witness input)
+	0xff, 0xff, 0xff, 0xff, // Sequence
+	0x1,                                    // Varint for number of outputs
+	0xb, 0x7, 0x6, 0x0, 0x0, 0x0, 0x0, 0x0, // Output amount
+	0x16, // Varint for length of pk script
+	0x0,  // Version 0 witness program
+	0x14, // OP_DATA_20
+	0x9d, 0xda, 0xc6, 0xf3, 0x9d, 0x51, 0xe0, 0x39,
+	0x8e, 0x53, 0x2a, 0x22, 0xc4, 0x1b, 0xa1, 0x89,
+	0x40, 0x6a, 0x85, 0x23, // 20-byte pub key hash
+	0x00,               // No item on the witness stack for the first input
+	0x00,               // No item on the witness stack for the second input
+	0x0, 0x0, 0x0, 0x0, // Lock time
+}
+
 // multiWitnessTxEncoded is the wire encoded bytes for multiWitnessTx including inputs
 // with witness data using protocol version 70012 and is used in the various
 // tests.