From 5cc32bbfc7dff4a672161d4b16a2fcb82cd031fc Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Thu, 24 Oct 2013 23:13:51 -0500 Subject: [PATCH] Add bounds checking to all variable length allocs. Several of the bitcoin data structures contain variable length entries, many of which have well-defined maximum limits. However, there are still a few cases, such as variable length strings and number of transactions which don't have clearly defined maximum limits. Instead they are only limited by the maximum size of a message. In order to efficiently decode messages, space is pre-allocated for the slices which hold these variable length pieces as to avoid needing to dynamically grow the backing arrays. Due to this however, it was previously possible to claim extremely high slice lengths which exceed available memory (or maximum allowed slice lengths). This commit imposes limits to all of these cases based on calculating the maximum possible number of elements that could fit into a message and using those as sane upper limits. The variable length string case was found (and tests added to hit it) by drahn@ which prompted an audit to find all cases. --- common.go | 21 ++++++++++++-- msgblock.go | 30 +++++++++++++++++-- msgtx.go | 83 ++++++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 121 insertions(+), 13 deletions(-) diff --git a/common.go b/common.go index 1d2b37ac..27923cfe 100644 --- a/common.go +++ b/common.go @@ -8,6 +8,7 @@ import ( "crypto/rand" "crypto/sha256" "encoding/binary" + "fmt" "io" "math" ) @@ -121,13 +122,27 @@ func writeVarInt(w io.Writer, pver uint32, val uint64) error { // readVarString reads a variable length string from r and returns it as a Go // string. A varString is encoded as a varInt containing the length of the -// string, and the bytes that represent the string itself. +// string, and the bytes that represent the string itself. An error is returned +// if the length is greater than the maximum block payload size, since it would +// not be possible to put a varString of that size into a block anyways and it +// also helps protect against memory exhuastion attacks and forced panics +// through malformed messages. func readVarString(r io.Reader, pver uint32) (string, error) { - slen, err := readVarInt(r, pver) + count, err := readVarInt(r, pver) if err != nil { return "", err } - buf := make([]byte, slen) + + // Prevent variable length strings that are larger than the maximum + // message size. It would be possible to cause memory exhaustion and + // panics without a sane upper bound on this count. + if count > maxMessagePayload { + str := fmt.Sprintf("variable length string is too long "+ + "[count %d, max %d]", count, maxMessagePayload) + return "", messageError("readVarString", str) + } + + buf := make([]byte, count) err = readElement(r, buf) if err != nil { return "", err diff --git a/msgblock.go b/msgblock.go index 7ad79a3e..35693211 100644 --- a/msgblock.go +++ b/msgblock.go @@ -6,6 +6,7 @@ package btcwire import ( "bytes" + "fmt" "io" ) @@ -22,6 +23,10 @@ const MaxBlocksPerMsg = 500 // MaxBlockPayload is the maximum bytes a block message can be in bytes. const MaxBlockPayload = 1000000 // Not actually 1MB which would be 1024 * 1024 +// maxTxPerBlock is the maximum number of transactions that could +// possibly fit into a block. +const maxTxPerBlock = (MaxBlockPayload / minTxPayload) + 1 + // TxLoc holds locator data for the offset and length of where a transaction is // located within a MsgBlock data buffer. type TxLoc struct { @@ -72,8 +77,18 @@ func (msg *MsgBlock) BtcDecode(r io.Reader, pver uint32) error { return err } - msg.Transactions = make([]*MsgTx, 0, msg.Header.TxnCount) - for i := uint64(0); i < msg.Header.TxnCount; i++ { + // Prevent more transactions than could possibly fit into a block. + // It would be possible to cause memory exhaustion and panics without + // a sane upper bound on this count. + txCount := msg.Header.TxnCount + if txCount > maxTxPerBlock { + str := fmt.Sprintf("too many transactions to fit into a block "+ + "[count %d, max %d]", txCount, maxTxPerBlock) + return messageError("MsgBlock.BtcDecode", str) + } + + msg.Transactions = make([]*MsgTx, 0, txCount) + for i := uint64(0); i < txCount; i++ { tx := MsgTx{} err := tx.BtcDecode(r, pver) if err != nil { @@ -115,9 +130,18 @@ func (msg *MsgBlock) DeserializeTxLoc(r *bytes.Buffer) ([]TxLoc, error) { return nil, err } + // Prevent more transactions than could possibly fit into a block. + // It would be possible to cause memory exhaustion and panics without + // a sane upper bound on this count. + txCount := msg.Header.TxnCount + if txCount > maxTxPerBlock { + str := fmt.Sprintf("too many transactions to fit into a block "+ + "[count %d, max %d]", txCount, maxTxPerBlock) + return nil, messageError("MsgBlock.DeserializeTxLoc", str) + } + // Deserialize each transaction while keeping track of its location // within the byte stream. - txCount := msg.Header.TxnCount msg.Transactions = make([]*MsgTx, 0, txCount) txLocs := make([]TxLoc, txCount) for i := uint64(0); i < txCount; i++ { diff --git a/msgtx.go b/msgtx.go index cdccbcc7..673faf1f 100644 --- a/msgtx.go +++ b/msgtx.go @@ -6,9 +6,17 @@ package btcwire import ( "bytes" + "fmt" "io" ) +// TxVersion is the current latest supported transaction version. +const TxVersion = 1 + +// MaxTxInSequenceNum is the maximum sequence number the sequence field +// of a transaction input can be. +const MaxTxInSequenceNum uint32 = 0xffffffff + // defaultTxInOutAlloc is the default size used for the backing array for // transaction inputs and outputs. The array will dynamically grow as needed, // but this figure is intended to provide enough space for the number of @@ -16,12 +24,33 @@ import ( // backing array multiple times. const defaultTxInOutAlloc = 15 -// TxVersion is the current latest supported transaction version. -const TxVersion = 1 +const ( + // minTxInPayload is the minimum payload size for a transaction input. + // PreviousOutpoint.Hash + PreviousOutpoint.Index 4 bytes + Varint for + // SignatureScript length 1 byte + Sequence 4 bytes. + minTxInPayload = 9 + HashSize -// MaxTxInSequenceNum is the maximum sequence number the sequence field -// of a transaction input can be. -const MaxTxInSequenceNum uint32 = 0xffffffff + // maxTxInPerMessage is the maximum number of transactions inputs that + // a transaction which fits into a message could possibly have. + maxTxInPerMessage = (maxMessagePayload / minTxInPayload) + 1 + + // minTxOutPayload is the minimum payload size for a transaction output. + // Value 8 bytes + Varint for PkScript length 1 byte. + minTxOutPayload = 9 + + // maxTxOutPerMessage is the maximum number of transactions outputs that + // a transaction which fits into a message could possibly have. + maxTxOutPerMessage = (maxMessagePayload / minTxOutPayload) + 1 + + // minTxPayload is the minimum payload size for a transaction. Note + // that any realistically usable transaction must have at least one + // input or output, but that is a rule enforced at a higher layer, so + // it is intentionally not included here. + // Version 4 bytes + Varint number of transaction inputs 1 byte + Varint + // number of transaction outputs 1 byte + LockTime 4 bytes + min input + // payload + min output payload. + minTxPayload = 10 +) // OutPoint defines a bitcoin data type that is used to track previous // transaction outputs. @@ -191,6 +220,16 @@ func (msg *MsgTx) BtcDecode(r io.Reader, pver uint32) error { return err } + // Prevent more input transactions than could possibly fit into a + // message. It would be possible to cause memory exhaustion and panics + // without a sane upper bound on this count. + if count > uint64(maxTxInPerMessage) { + str := fmt.Sprintf("too many input transactions to fit into "+ + "max message size [count %d, max %d]", count, + maxTxInPerMessage) + return messageError("MsgTx.BtcDecode", str) + } + msg.TxIn = make([]*TxIn, 0, count) for i := uint64(0); i < count; i++ { ti := TxIn{} @@ -206,6 +245,16 @@ func (msg *MsgTx) BtcDecode(r io.Reader, pver uint32) error { return err } + // Prevent more output transactions than could possibly fit into a + // message. It would be possible to cause memory exhaustion and panics + // without a sane upper bound on this count. + if count > uint64(maxTxOutPerMessage) { + str := fmt.Sprintf("too many output transactions to fit into "+ + "max message size [count %d, max %d]", count, + maxTxOutPerMessage) + return messageError("MsgTx.BtcDecode", str) + } + msg.TxOut = make([]*TxOut, 0, count) for i := uint64(0); i < count; i++ { to := TxOut{} @@ -362,6 +411,16 @@ func readTxIn(r io.Reader, pver uint32, version uint32, ti *TxIn) error { return err } + // Prevent signature script 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(maxMessagePayload) { + str := fmt.Sprintf("transaction input signature script is "+ + "larger than max message size [count %d, max %d]", + count, maxMessagePayload) + return messageError("MsgTx.BtcDecode", str) + } + b := make([]byte, count) err = readElement(r, b) if err != nil { @@ -413,12 +472,22 @@ func readTxOut(r io.Reader, pver uint32, version uint32, to *TxOut) error { return err } - slen, err := readVarInt(r, pver) + count, err := readVarInt(r, pver) if err != nil { return err } - b := make([]byte, slen) + // Prevent public key script 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(maxMessagePayload) { + str := fmt.Sprintf("transaction output public key script is "+ + "larger than max message size [count %d, max %d]", + count, maxMessagePayload) + return messageError("MsgTx.BtcDecode", str) + } + + b := make([]byte, count) err = readElement(r, b) if err != nil { return err