diff --git a/baselib/actor/ask_response.go b/baselib/actor/ask_response.go index e7dcc2ca8..175c7fb35 100644 --- a/baselib/actor/ask_response.go +++ b/baselib/actor/ask_response.go @@ -108,7 +108,15 @@ func (m *AskResponse) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the framing before decoding so a crafted durable payload cannot + // drive an unbounded make() inside the tlv decoder (ResultBlob is a + // []byte record sized from its declared length). + safe, err := safeActorTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return err } diff --git a/baselib/actor/decode_fuzz_test.go b/baselib/actor/decode_fuzz_test.go new file mode 100644 index 000000000..e40cb4271 --- /dev/null +++ b/baselib/actor/decode_fuzz_test.go @@ -0,0 +1,126 @@ +package actor + +import ( + "bytes" + "testing" + "time" + + "github.com/lightningnetwork/lnd/fn/v2" +) + +// These fuzz tests target the actor-framework TLV decoders, which run on +// attacker-controlled bytes replayed from durable mailboxes across rolling +// upgrades. The contract is uniform: a decoder MUST return an error (never +// panic, OOM, or slice-OOB) on malformed input, and any value it accepts +// MUST round-trip back through its encoder. + +// FuzzAskResponseDecode exercises the durable AskResponse decoder. Its +// ResultBlob is a []byte record sized from a declared length, the unbounded +// make() sink the framing pre-validation now bounds. +func FuzzAskResponseDecode(f *testing.F) { + var buf bytes.Buffer + seed := AskResponse{ + CorrelationID: "corr-1", + ResultBlob: []byte{1, 2, 3, 4}, + ErrorText: "boom", + } + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + + // A []byte record (type 2) declaring a near-2^64 length in a tiny + // envelope: the historical makeslice crasher. + f.Add([]byte{ + 0x02, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + var v AskResponse + if err := v.Decode(bytes.NewReader(data)); err != nil { + return + } + + var out bytes.Buffer + if err := v.Encode(&out); err != nil { + t.Fatalf("re-encode: %v", err) + } + + var v2 AskResponse + if err := v2.Decode(bytes.NewReader(out.Bytes())); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzRestartMessageDecode exercises the RestartMessage checkpoint decoder. +// Its actorID/stateType/stateData are []byte records sized from declared +// lengths. +func FuzzRestartMessageDecode(f *testing.F) { + var buf bytes.Buffer + seed := &RestartMessage{ + Checkpoint: fn.Some(Checkpoint{ + ActorID: "actor-1", + StateType: "state", + StateData: []byte{9, 8, 7}, + Version: 3, + UpdatedAt: time.Unix(1000, 0), + }), + } + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + f.Add([]byte{ + 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + var v RestartMessage + if err := v.Decode(bytes.NewReader(data)); err != nil { + return + } + + var out bytes.Buffer + if err := v.Encode(&out); err != nil { + t.Fatalf("re-encode: %v", err) + } + + var v2 RestartMessage + if err := v2.Decode(bytes.NewReader(out.Bytes())); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzMessageCodecDecode exercises the top-level envelope decoder that every +// durable mailbox message flows through. Its payload is read into +// make([]byte, payloadLen) sized from a declared length, the most reachable +// unbounded-alloc sink in the actor framework. +func FuzzMessageCodecDecode(f *testing.F) { + codec := NewMessageCodec() + codec.MustRegister(AskResponseMsgType, func() TLVMessage { + return &AskResponse{} + }) + + resp := &AskResponse{ + CorrelationID: "c", + ResultBlob: []byte{1, 2}, + } + if raw, err := codec.Encode(resp); err == nil { + f.Add(raw) + } + f.Add([]byte{}) + + // type=AskResponseMsgType varint, then a near-2^64 payload length in a + // tiny envelope: the historical makeslice crasher at the codec layer. + f.Add([]byte{ + 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + // The codec round-trip is not byte-exact across all registered + // types, so assert only the no-panic decode contract here. + _, _ = codec.Decode(data) + }) +} diff --git a/baselib/actor/restart.go b/baselib/actor/restart.go index 3bdc5f9d2..fdf5337bd 100644 --- a/baselib/actor/restart.go +++ b/baselib/actor/restart.go @@ -105,8 +105,17 @@ func (m *RestartMessage) Decode(r io.Reader) error { return err } + // Bound the framing before decoding so a crafted durable checkpoint + // payload cannot drive an unbounded make() inside the tlv decoder + // (actorID/stateType/stateData are []byte records sized from their + // declared lengths). + safe, err := safeActorTLVReader(r) + if err != nil { + return err + } + // Decode and get typeMap showing which fields were present. - typeMap, err := stream.DecodeWithParsedTypes(r) + typeMap, err := stream.DecodeWithParsedTypes(safe) if err != nil { return err } diff --git a/baselib/actor/tlv_bounds.go b/baselib/actor/tlv_bounds.go new file mode 100644 index 000000000..57948398f --- /dev/null +++ b/baselib/actor/tlv_bounds.go @@ -0,0 +1,87 @@ +package actor + +import ( + "bytes" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// maxActorMessageSize bounds the total wire size of an untrusted actor TLV +// message (the codec envelope and every message payload decoded from a +// durable mailbox) before any individual record length is honored. Actor +// payloads can embed domain snapshots and PSBT-bearing blobs, so the cap is +// generous at 16 MiB while still rejecting the unbounded-allocation DoS +// where a tiny payload declares a multi-gigabyte (or near-2^64) length. +// Durable-mailbox bytes are replayed from disk across rolling upgrades and +// can cross the cross-actor trust boundary, so the bound is load-bearing on +// both the live and replay paths. +const maxActorMessageSize = 16 << 20 + +// validateTLVRecordLengths walks the (type, length, value) framing of a +// buffered TLV stream and rejects any record whose declared length exceeds +// the bytes remaining in the buffer. It deliberately does not interpret +// record contents; it only ensures the framing cannot drive an over-sized +// make([]byte, declaredLength) in the real tlv decoder before any value +// bytes are read. +// +// The tlv library sizes its value buffers from the declared length before +// reading any value bytes (stream.go's unknown-record path and +// primitive.go's DVarBytes), and the non-P2P Decode path applies no length +// bound. A producer-declared length near 2^64 panics the decoder with +// "makeslice: cap out of range" and a multi-gigabyte length drives an OOM. +func validateTLVRecordLengths(buf []byte) error { + var scratch [8]byte + br := bytes.NewReader(buf) + + for br.Len() > 0 { + if _, err := tlv.ReadVarInt(br, &scratch); err != nil { + return fmt.Errorf("read tlv type: %w", err) + } + + length, err := tlv.ReadVarInt(br, &scratch) + if err != nil { + return fmt.Errorf("read tlv length: %w", err) + } + + // A record can never carry more value bytes than remain in the + // buffer; reject before the decoder allocates. + if length > uint64(br.Len()) { + return fmt.Errorf("tlv record length %d exceeds %d "+ + "remaining bytes", length, br.Len()) + } + + if _, err := br.Seek(int64(length), io.SeekCurrent); err != nil { + return fmt.Errorf("skip tlv value: %w", err) + } + } + + return nil +} + +// safeActorTLVReader reads an untrusted actor TLV payload into a size-capped +// buffer and pre-validates its framing so the downstream +// tlv.Stream.DecodeWithParsedTypes can never be handed a record length +// larger than the bytes physically present. See validateTLVRecordLengths +// for why this is required. +func safeActorTLVReader(r io.Reader) (*bytes.Reader, error) { + // Read at most maxActorMessageSize+1 so an over-cap message is detected + // without buffering an unbounded amount. + limited := io.LimitReader(r, maxActorMessageSize+1) + buf, err := io.ReadAll(limited) + if err != nil { + return nil, fmt.Errorf("read actor message: %w", err) + } + + if len(buf) > maxActorMessageSize { + return nil, fmt.Errorf("actor message %d bytes exceeds max %d", + len(buf), maxActorMessageSize) + } + + if err := validateTLVRecordLengths(buf); err != nil { + return nil, err + } + + return bytes.NewReader(buf), nil +} diff --git a/baselib/actor/tlv_message.go b/baselib/actor/tlv_message.go index 4da1d047b..235ff0b9b 100644 --- a/baselib/actor/tlv_message.go +++ b/baselib/actor/tlv_message.go @@ -147,6 +147,16 @@ func (c *MessageCodec) Decode(data []byte) (TLVMessage, error) { return nil, fmt.Errorf("read payload length: %w", err) } + // Reject a declared payload length that cannot be backed by the bytes + // physically present before allocating make([]byte, payloadLen). The + // envelope is sourced from the durable mailbox, so a corrupt or + // malicious length near 2^64 would otherwise panic with "makeslice: + // cap out of range" or drive an OOM here, before io.ReadFull ever runs. + if payloadLen > uint64(r.Len()) { + return nil, fmt.Errorf("payload length %d exceeds %d remaining "+ + "bytes", payloadLen, r.Len()) + } + // Read the payload. payload := make([]byte, payloadLen) if _, err := io.ReadFull(r, payload); err != nil { diff --git a/db/tree_codec.go b/db/tree_codec.go index 927ea9e54..4cbda40c6 100644 --- a/db/tree_codec.go +++ b/db/tree_codec.go @@ -44,6 +44,107 @@ const ( MaxTreeChildrenPerNode = 64 ) +// maxTreeBlobSize bounds the total wire size of an untrusted tree or +// node TLV blob before any individual record length is honored. Tree +// blobs are fed to DeserializeTree from the durable mailbox, persisted +// rows, and operator-supplied indexer responses, all of which are +// untrusted. A production tree's serialized size is dominated by per +// node 36-byte outpoints, 36-byte+ outputs, 33-byte cosigner keys, and +// 64-byte signatures across a binary tree whose practical depth is well +// under MaxTreeDeserializeDepth; 8 MiB gives generous headroom while +// still rejecting the unbounded-allocation DoS where a tiny payload +// declares a multi-gigabyte (or near-2^64) DVarBytes record length. +const maxTreeBlobSize = 8 << 20 + +// safeTreeReader reads an untrusted tree/node TLV blob into a size +// capped buffer and pre-validates the (type, length, value) framing so +// the downstream tlv.Stream decode can never be handed a record length +// larger than the bytes physically present. The tlv library sizes its +// DVarBytes / tlv.Blob value buffers with make([]byte, declaredLength) +// before reading any value bytes, so a producer-declared length near +// 2^64 panics the decoder ("makeslice: len out of range") and a +// multi-gigabyte length OOMs -- both reachable from a few bytes of a +// crafted tree blob replayed from disk. Bounding each record length +// against the remaining buffer caps every allocation at the blob size, +// itself capped at maxTreeBlobSize. +func safeTreeReader(r io.Reader) (io.Reader, error) { + limited := io.LimitReader(r, maxTreeBlobSize+1) + + buf, err := io.ReadAll(limited) + if err != nil { + return nil, fmt.Errorf("read tree blob: %w", err) + } + + if len(buf) > maxTreeBlobSize { + return nil, fmt.Errorf("tree blob %d bytes exceeds max %d", + len(buf), maxTreeBlobSize) + } + + if err := validateTreeRecordLengths(buf); err != nil { + return nil, err + } + + return bytes.NewReader(buf), nil +} + +// validateTreeRecordLengths walks the (type, length, value) framing of +// a buffered TLV stream and rejects any record whose declared length +// exceeds the bytes remaining in the buffer. It does not interpret +// record contents; it only ensures the framing cannot drive an +// over-sized make() in the real decoder. +func validateTreeRecordLengths(buf []byte) error { + var scratch [8]byte + br := bytes.NewReader(buf) + + for br.Len() > 0 { + if _, err := tlv.ReadVarInt(br, &scratch); err != nil { + return fmt.Errorf("read tlv type: %w", err) + } + + length, err := tlv.ReadVarInt(br, &scratch) + if err != nil { + return fmt.Errorf("read tlv length: %w", err) + } + + if length > uint64(br.Len()) { + return fmt.Errorf("tlv record length %d exceeds %d "+ + "remaining bytes", length, br.Len()) + } + + if _, err := br.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return fmt.Errorf("skip tlv value: %w", err) + } + } + + return nil +} + +// checkElemCount rejects an element count whose minimum on-wire +// footprint (count * minBytesPerElem) exceeds the declared record +// length before any make([]T, count) runs. The tlv library does not +// cap a record's element count on the non-p2p decode path, so a +// varint-driven count near 2^64 would otherwise panic the decoder with +// "makeslice: len out of range" or drive an OOM. A record can never +// legitimately describe more elements than its own length can hold, so +// bounding the count against recordLen caps every slice allocation at +// the record size. recordLen is the per-record length the tlv stream +// frames and hands to the decoder, itself bounded by the enclosing +// node blob. +func checkElemCount(count, minBytesPerElem, recordLen uint64) error { + if minBytesPerElem == 0 { + return fmt.Errorf("min bytes per element must be positive") + } + + if count > recordLen/minBytesPerElem { + return fmt.Errorf("element count %d exceeds %d bytes of "+ + "record capacity", count, recordLen) + } + + return nil +} + // TLV type aliases for Node serialization. type ( tlvNodeInput = tlv.TlvType0 @@ -161,6 +262,14 @@ func txOutDecoder(r io.Reader, val interface{}, buf *[8]byte, l uint64) error { return err } + // Bound the script length against the record length before + // make([]byte, scriptLen): a crafted varint could otherwise + // drive an OOM ahead of io.ReadFull. + if scriptLen > l { + return fmt.Errorf("script length %d exceeds record "+ + "length %d", scriptLen, l) + } + t.PkScript = make([]byte, scriptLen) _, err = io.ReadFull(r, t.PkScript) @@ -235,6 +344,17 @@ func txOutsDecoder(r io.Reader, val interface{}, buf *[8]byte, l uint64) error { return err } + // Each output costs at least 8 bytes (value) plus a 1-byte + // minimum varint script length, so bound numOutputs against + // the record length before make([]*wire.TxOut, numOutputs). + // Without this a varint-driven numOutputs near 2^64 panics the + // decoder ("makeslice: len out of range") or OOMs, reachable + // from a tiny untrusted node blob replayed from the durable + // mailbox. + if err := checkElemCount(numOutputs, 9, l); err != nil { + return fmt.Errorf("tx outputs: %w", err) + } + t.Outputs = make([]*wire.TxOut, numOutputs) for i := uint64(0); i < numOutputs; i++ { if _, err := io.ReadFull(r, buf[:]); err != nil { @@ -248,6 +368,13 @@ func txOutsDecoder(r io.Reader, val interface{}, buf *[8]byte, l uint64) error { return err } + // Bound the per-output script length against the record + // length before make([]byte, scriptLen). + if scriptLen > l { + return fmt.Errorf("output script length %d "+ + "exceeds record length %d", scriptLen, l) + } + pkScript := make([]byte, scriptLen) if _, err := io.ReadFull(r, pkScript); err != nil { return err @@ -314,6 +441,14 @@ func pubKeysDecoder(r io.Reader, val interface{}, buf *[8]byte, return err } + // Each compressed pubkey is exactly 33 bytes, so bound numKeys + // against the record length before make([]*btcec.PublicKey, + // numKeys): a varint-driven count near 2^64 would otherwise + // panic the decoder or OOM on a tiny untrusted node blob. + if err := checkElemCount(numKeys, 33, l); err != nil { + return fmt.Errorf("cosigner keys: %w", err) + } + p.Keys = make([]*btcec.PublicKey, numKeys) for i := uint64(0); i < numKeys; i++ { var keyBuf [33]byte @@ -541,7 +676,15 @@ func (t *tlvTree) Decode(r io.Reader) error { return err } - return stream.Decode(r) + // Bound the untrusted blob before decode so a crafted DVarBytes + // record length cannot drive an unbounded make() in the tlv + // library. + safe, err := safeTreeReader(r) + if err != nil { + return err + } + + return stream.Decode(safe) } // tlvNode is the TLV-serializable wrapper for tree.Node using RecordT. @@ -635,7 +778,16 @@ func (n *tlvNode) Decode(r io.Reader) error { return err } - _, err = stream.DecodeWithParsedTypes(r) + // Bound the untrusted node blob before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library (both + // the DVarBytes children blob and the per-record element counts in + // txOutsDecoder / pubKeysDecoder are sized from the wire). + safe, err := safeTreeReader(r) + if err != nil { + return err + } + + _, err = stream.DecodeWithParsedTypes(safe) if err != nil { return err } diff --git a/db/tree_codec_fuzz_test.go b/db/tree_codec_fuzz_test.go new file mode 100644 index 000000000..18b1fcf8c --- /dev/null +++ b/db/tree_codec_fuzz_test.go @@ -0,0 +1,91 @@ +package db + +import ( + "bytes" + "testing" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/darepo-client/lib/tree" +) + +// FuzzDeserializeTree drives DeserializeTree with arbitrary bytes. Tree +// blobs are fed from the durable mailbox, persisted rows, and +// operator-supplied indexer responses, all untrusted, so a crafted +// blob must surface as an error rather than panicking (regressions for +// the tlv unbounded-make DoS: DVarBytes SweepRoot/RootNode lengths, +// txOutsDecoder numOutputs, pubKeysDecoder numKeys, and per-output +// script lengths). DeserializeTree re-derives a tree.Tree but the +// serialized form is not guaranteed byte-stable across the map-keyed +// children, so a successful decode asserts re-serialize succeeds and +// re-deserializes cleanly rather than byte equality. +func FuzzDeserializeTree(f *testing.F) { + // Seed with a valid minimal tree so the fuzzer mutates real + // framing. + minimal := &tree.Tree{ + BatchOutpoint: wire.OutPoint{}, + Root: &tree.Node{ + Input: wire.OutPoint{}, + Outputs: []*wire.TxOut{}, + CoSigners: []*btcec.PublicKey{}, + Children: make(map[uint32]*tree.Node), + }, + } + if data, err := SerializeTree(minimal); err == nil { + f.Add(data) + } + + // Seed with a tree carrying an output and a cosigner key so the + // element-count decoders are reachable from the corpus. + withData := &tree.Tree{ + BatchOutpoint: wire.OutPoint{Hash: chainhash.Hash{0x01}}, + Root: &tree.Node{ + Input: wire.OutPoint{Hash: chainhash.Hash{0x02}}, + Outputs: []*wire.TxOut{ + {Value: 1000, PkScript: bytes.Repeat( + []byte{0x51}, 34, + )}, + }, + CoSigners: []*btcec.PublicKey{}, + Children: make(map[uint32]*tree.Node), + }, + } + if data, err := SerializeTree(withData); err == nil { + f.Add(data) + } + + // Known regression crashers reaching the outer DVarBytes and the + // inner element-count makes. + f.Add([]byte{ + // SweepRoot (type 2) with a near-2^63 declared length. + 0x02, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + f.Add([]byte{ + // RootNodeData (type 3) holding a node blob whose outputs + // record (type 1) declares a huge numOutputs. + 0x03, 0x0b, + 0x01, 0x09, + 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + f.Add([]byte{}) + + f.Fuzz(func(t *testing.T, data []byte) { + // MUST NOT panic on any input. + tr, err := DeserializeTree(data) + if err != nil { + return + } + + // A successfully decoded tree must re-serialize and + // re-deserialize cleanly. + out, err := SerializeTree(tr) + if err != nil { + t.Fatalf("re-serialize: %v", err) + } + + if _, err := DeserializeTree(out); err != nil { + t.Fatalf("re-deserialize: %v", err) + } + }) +} diff --git a/internal/actortest/counter_messages.go b/internal/actortest/counter_messages.go index 6e6d32959..d5c94e36d 100644 --- a/internal/actortest/counter_messages.go +++ b/internal/actortest/counter_messages.go @@ -80,7 +80,17 @@ func (m *IncrementMsg) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the framing before decoding. Even though this message only + // holds a fixed-width scalar, the tlv decoder's unknown-record path + // allocates make([]byte, declaredLength) for any unrecognized odd + // type, so a crafted payload with a huge unknown-record length would + // otherwise panic the decoder. + safe, err := safeCounterTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return err } @@ -136,7 +146,15 @@ func (m *DecrementMsg) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the framing before decoding so a crafted unknown-record length + // cannot drive an unbounded make() inside the tlv decoder (see + // IncrementMsg.Decode for the unknown-record-path rationale). + safe, err := safeCounterTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return err } @@ -231,7 +249,15 @@ func (m *ForwardMsg) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the framing before decoding so a crafted durable payload cannot + // drive an unbounded make() inside the tlv decoder (target and payload + // are []byte records sized from their declared lengths). + safe, err := safeCounterTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return err } diff --git a/internal/actortest/decode_fuzz_test.go b/internal/actortest/decode_fuzz_test.go new file mode 100644 index 000000000..d8a5a3755 --- /dev/null +++ b/internal/actortest/decode_fuzz_test.go @@ -0,0 +1,70 @@ +package actortest + +import ( + "bytes" + "testing" +) + +// FuzzForwardMsgDecode exercises the ForwardMsg decoder. Its Target and +// Payload are []byte records sized from declared lengths, the unbounded +// make() sink the framing pre-validation now bounds. ForwardMsg flows +// through the durable outbox/mailbox path in the actor integration tests. +func FuzzForwardMsgDecode(f *testing.F) { + var buf bytes.Buffer + seed := &ForwardMsg{ + Target: "actor-b", + MsgType: IncrementMsgType, + Payload: []byte{1, 2, 3}, + } + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + + // A []byte record (type 4 payload) declaring a near-2^64 length in a + // tiny envelope: the historical makeslice crasher. + f.Add([]byte{ + 0x04, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + var v ForwardMsg + if err := v.Decode(bytes.NewReader(data)); err != nil { + return + } + + var out bytes.Buffer + if err := v.Encode(&out); err != nil { + t.Fatalf("re-encode: %v", err) + } + + var v2 ForwardMsg + if err := v2.Decode(bytes.NewReader(out.Bytes())); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzIncrementMsgDecode exercises the IncrementMsg decoder. It carries a +// single uint64 scalar record; the test asserts the no-panic decode +// contract on arbitrary bytes. +func FuzzIncrementMsgDecode(f *testing.F) { + var buf bytes.Buffer + seed := &IncrementMsg{Amount: 42} + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + + f.Fuzz(func(t *testing.T, data []byte) { + var v IncrementMsg + if err := v.Decode(bytes.NewReader(data)); err != nil { + return + } + + var out bytes.Buffer + if err := v.Encode(&out); err != nil { + t.Fatalf("re-encode: %v", err) + } + }) +} diff --git a/internal/actortest/testdata/fuzz/FuzzIncrementMsgDecode/6dd7e93fb2c59c26 b/internal/actortest/testdata/fuzz/FuzzIncrementMsgDecode/6dd7e93fb2c59c26 new file mode 100644 index 000000000..0b6a11e4d --- /dev/null +++ b/internal/actortest/testdata/fuzz/FuzzIncrementMsgDecode/6dd7e93fb2c59c26 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("0\xff00000000") diff --git a/internal/actortest/tlv_bounds.go b/internal/actortest/tlv_bounds.go new file mode 100644 index 000000000..852a6bdaa --- /dev/null +++ b/internal/actortest/tlv_bounds.go @@ -0,0 +1,60 @@ +package actortest + +import ( + "bytes" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// maxCounterMessageSize bounds the wire size of an untrusted counter test +// message before any record length is honored. Counter messages are tiny +// (a scalar plus a small target/payload), so 1 MiB is generous while +// rejecting the unbounded-allocation DoS where a tiny payload declares a +// multi-gigabyte (or near-2^64) record length. +const maxCounterMessageSize = 1 << 20 + +// safeCounterTLVReader reads an untrusted counter TLV payload into a +// size-capped buffer and pre-validates its (type, length) framing so the +// downstream tlv.Stream.DecodeWithParsedTypes can never be handed a record +// length larger than the bytes physically present. The tlv library sizes +// its value buffers from the declared length before reading any value bytes +// and applies no length bound on the non-P2P Decode path, so a crafted +// payload would otherwise panic or OOM the decoder. +func safeCounterTLVReader(r io.Reader) (*bytes.Reader, error) { + limited := io.LimitReader(r, maxCounterMessageSize+1) + buf, err := io.ReadAll(limited) + if err != nil { + return nil, fmt.Errorf("read counter message: %w", err) + } + + if len(buf) > maxCounterMessageSize { + return nil, fmt.Errorf("counter message %d bytes exceeds max %d", + len(buf), maxCounterMessageSize) + } + + var scratch [8]byte + br := bytes.NewReader(buf) + for br.Len() > 0 { + if _, err := tlv.ReadVarInt(br, &scratch); err != nil { + return nil, fmt.Errorf("read tlv type: %w", err) + } + + length, err := tlv.ReadVarInt(br, &scratch) + if err != nil { + return nil, fmt.Errorf("read tlv length: %w", err) + } + + if length > uint64(br.Len()) { + return nil, fmt.Errorf("tlv record length %d exceeds %d "+ + "remaining bytes", length, br.Len()) + } + + if _, err := br.Seek(int64(length), io.SeekCurrent); err != nil { + return nil, fmt.Errorf("skip tlv value: %w", err) + } + } + + return bytes.NewReader(buf), nil +} diff --git a/ledger/messages.go b/ledger/messages.go index 209a5b6ee..bd6a796d1 100644 --- a/ledger/messages.go +++ b/ledger/messages.go @@ -1,6 +1,7 @@ package ledger import ( + "bytes" "encoding/binary" "fmt" "io" @@ -127,6 +128,98 @@ func decodeFixedBytes(field string, got []byte, want int) error { return nil } +// maxLedgerMessageSize bounds the total wire size of an untrusted +// ledger TLV message before any record length is honored. Every ledger +// message is dominated by fixed-width identifiers (16/32-byte IDs), a +// handful of uint32/uint64 scalars, and a few short string fields, so +// 1 MiB gives generous headroom while rejecting the unbounded +// allocation DoS where a malicious payload declares a multi-gigabyte +// (or near-2^64) TLV record length in a tiny envelope. These messages +// persist in the durable mailbox and are replayed from disk across +// upgrades. See safeTLVReader for why the cap is load-bearing. +const maxLedgerMessageSize = 1 << 20 + +// safeTLVReader reads an untrusted TLV payload into a size-capped +// buffer and pre-validates the (type, length, value) framing so the +// downstream tlv.Stream.DecodeWithParsedTypes can never be handed a +// record length larger than the bytes physically present. +// +// This is load-bearing because the tlv library sizes its value buffers +// with make([]byte, declaredLength) BEFORE reading any value bytes: +// stream.go does bytes.NewBuffer(make([]byte, 0, length)) for unknown +// records, and primitive.go's DVarBytes does make([]byte, l) for known +// []byte fields (FeeType, Source, Classification, IdempotencyKey here). +// A producer-declared length near 2^64 panics the decoder with +// "makeslice: cap out of range" and a multi-gigabyte length OOMs -- +// both reachable from a few attacker-controlled bytes that cross the +// trust boundary and persist in the durable mailbox. By rejecting any +// record whose declared length exceeds the remaining buffer we cap +// every allocation at the message size, itself capped at +// maxLedgerMessageSize. +func safeTLVReader(r io.Reader) (io.Reader, error) { + // Read at most maxLedgerMessageSize+1 so an over-cap message is + // detected without buffering an unbounded amount. + limited := io.LimitReader(r, maxLedgerMessageSize+1) + + buf, err := io.ReadAll(limited) + if err != nil { + return nil, fmt.Errorf("%w: read ledger message: %v", + ErrInvalidMessage, err) + } + + if len(buf) > maxLedgerMessageSize { + return nil, fmt.Errorf("%w: message %d bytes exceeds max %d", + ErrInvalidMessage, len(buf), maxLedgerMessageSize) + } + + if err := validateTLVRecordLengths(buf); err != nil { + return nil, err + } + + return bytes.NewReader(buf), nil +} + +// validateTLVRecordLengths walks the (type, length, value) framing of a +// buffered TLV stream and rejects any record whose declared length +// exceeds the bytes remaining in the buffer. It deliberately does not +// interpret record contents; it only ensures the framing cannot drive +// an over-sized make() in the real decoder. +func validateTLVRecordLengths(buf []byte) error { + var scratch [8]byte + br := bytes.NewReader(buf) + + for br.Len() > 0 { + // The type varint may legitimately end the stream cleanly. + if _, err := tlv.ReadVarInt(br, &scratch); err != nil { + return fmt.Errorf("%w: read tlv type: %v", + ErrInvalidMessage, err) + } + + length, err := tlv.ReadVarInt(br, &scratch) + if err != nil { + return fmt.Errorf("%w: read tlv length: %v", + ErrInvalidMessage, err) + } + + // A record can never carry more value bytes than remain in + // the buffer; reject before the decoder allocates. + if length > uint64(br.Len()) { + return fmt.Errorf("%w: tlv record length %d exceeds "+ + "%d remaining bytes", ErrInvalidMessage, length, + br.Len()) + } + + if _, err := br.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return fmt.Errorf("%w: skip tlv value: %v", + ErrInvalidMessage, err) + } + } + + return nil +} + // TLV type constants for client-side ledger actor messages. // These use the 0x9xxx range to avoid collisions with the // server-side ledger actor (0x8xxx) and other actor subsystems. @@ -327,7 +420,14 @@ func (m *FeePaidMsg) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return fmt.Errorf("decode FeePaidMsg: %w", err) } @@ -467,7 +567,14 @@ func (m *VTXOReceivedMsg) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return fmt.Errorf("decode VTXOReceivedMsg: %w", err) } @@ -591,7 +698,14 @@ func (m *VTXOSentMsg) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return fmt.Errorf("decode VTXOSentMsg: %w", err) } @@ -718,7 +832,14 @@ func (m *ExitCostMsg) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return fmt.Errorf("decode ExitCostMsg: %w", err) } @@ -846,7 +967,14 @@ func (m *UTXOCreatedMsg) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return fmt.Errorf("decode UTXOCreatedMsg: %w", err) } @@ -972,7 +1100,14 @@ func (m *UTXOSpentMsg) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return fmt.Errorf("decode UTXOSpentMsg: %w", err) } diff --git a/ledger/messages_fuzz_test.go b/ledger/messages_fuzz_test.go new file mode 100644 index 000000000..2a9000eb7 --- /dev/null +++ b/ledger/messages_fuzz_test.go @@ -0,0 +1,144 @@ +package ledger + +import ( + "bytes" + "testing" + + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/darepo-client/baselib/actor" +) + +// ledgerHugeRecordPayloads returns tiny TLV payloads that each declare +// a record length near 2^63 / 2^64. These are the canonical crashers +// for the tlv unbounded-make DoS and MUST be rejected (error) rather +// than panicking the ledger actor on durable replay. +func ledgerHugeRecordPayloads() [][]byte { + return [][]byte{ + // Unknown odd type 11, length ~2^63 (stream.go make path). + {0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + // Type 3 (a []byte/scalar field), giant declared length. + {0x03, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + // Type 9 (idempotency / classification []byte), giant length. + {0x09, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + {}, + } +} + +// fuzzLedgerRoundTrip runs the standard decode→encode→decode invariant +// for a TLV ledger message. The message must never panic on decode; a +// successful decode must re-encode and re-decode cleanly. msgFactory +// returns a fresh zero value each call so the two decode passes are +// independent. +func fuzzLedgerRoundTrip(f *testing.F, seed actor.TLVMessage, + msgFactory func() actor.TLVMessage) { + + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + for _, p := range ledgerHugeRecordPayloads() { + f.Add(p) + } + + f.Fuzz(func(t *testing.T, data []byte) { + v := msgFactory() + + // MUST NOT panic on any input. + if err := v.Decode(bytes.NewReader(data)); err != nil { + return + } + + var out bytes.Buffer + if err := v.Encode(&out); err != nil { + t.Fatalf("re-encode: %v", err) + } + + v2 := msgFactory() + if err := v2.Decode(bytes.NewReader(out.Bytes())); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzFeePaidMsgDecode fuzzes the FeePaidMsg TLV decoder. +func FuzzFeePaidMsgDecode(f *testing.F) { + seed := &FeePaidMsg{ + RoundID: [16]byte{0x01}, + AmountSat: 1234, + FeeType: "boarding_fee_paid", + BlockHeight: 100, + IdempotencyKey: bytes.Repeat([]byte{0x07}, 32), + } + fuzzLedgerRoundTrip(f, seed, func() actor.TLVMessage { + return &FeePaidMsg{} + }) +} + +// FuzzVTXOReceivedMsgDecode fuzzes the VTXOReceivedMsg TLV decoder. +func FuzzVTXOReceivedMsgDecode(f *testing.F) { + seed := &VTXOReceivedMsg{ + OutpointHash: [32]byte{0x02}, + OutpointIndex: 1, + AmountSat: 5000, + Source: "round", + RoundID: [16]byte{0x03}, + } + fuzzLedgerRoundTrip(f, seed, func() actor.TLVMessage { + return &VTXOReceivedMsg{} + }) +} + +// FuzzVTXOSentMsgDecode fuzzes the VTXOSentMsg TLV decoder, which +// carries a fixed-width outpoint record alongside scalar fields. +func FuzzVTXOSentMsgDecode(f *testing.F) { + seed := &VTXOSentMsg{ + SessionID: [32]byte{0x04}, + AmountSat: 6000, + Outpoint: wire.OutPoint{Index: 2}, + } + fuzzLedgerRoundTrip(f, seed, func() actor.TLVMessage { + return &VTXOSentMsg{} + }) +} + +// FuzzExitCostMsgDecode fuzzes the ExitCostMsg TLV decoder. +func FuzzExitCostMsgDecode(f *testing.F) { + seed := &ExitCostMsg{ + OutpointHash: [32]byte{0x05}, + OutpointIndex: 3, + AmountSat: 7000, + ExitCostSat: 42, + BlockHeight: 200, + } + fuzzLedgerRoundTrip(f, seed, func() actor.TLVMessage { + return &ExitCostMsg{} + }) +} + +// FuzzUTXOCreatedMsgDecode fuzzes the UTXOCreatedMsg TLV decoder. +func FuzzUTXOCreatedMsgDecode(f *testing.F) { + seed := &UTXOCreatedMsg{ + OutpointHash: [32]byte{0x06}, + OutpointIndex: 4, + AmountSat: 8000, + BlockHeight: 300, + Classification: "deposit", + } + fuzzLedgerRoundTrip(f, seed, func() actor.TLVMessage { + return &UTXOCreatedMsg{} + }) +} + +// FuzzUTXOSpentMsgDecode fuzzes the UTXOSpentMsg TLV decoder. +func FuzzUTXOSpentMsgDecode(f *testing.F) { + seed := &UTXOSpentMsg{ + OutpointHash: [32]byte{0x07}, + OutpointIndex: 5, + AmountSat: 9000, + BlockHeight: 400, + Classification: "round_funding", + } + fuzzLedgerRoundTrip(f, seed, func() actor.TLVMessage { + return &UTXOSpentMsg{} + }) +} diff --git a/lib/bip322/sig_fuzz_test.go b/lib/bip322/sig_fuzz_test.go new file mode 100644 index 000000000..a535281c0 --- /dev/null +++ b/lib/bip322/sig_fuzz_test.go @@ -0,0 +1,76 @@ +package bip322 + +import ( + "bytes" + "testing" + + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" +) + +// FuzzDecodeSig drives attacker-controlled bytes through the full-format +// BIP-322 signature decoder. A BIP-322 signature payload is supplied by the +// remote client and crosses the trust boundary as the serialized to_sign +// transaction, so the decoder must never panic from a malformed or +// length-inflated payload. +// +// DecodeSig parses the bytes via wire.MsgTx.Deserialize, which bounds its own +// input/output/witness counts (maxTxInPerMessage et al.) and reuses a bounded +// script pool, so the framing is hardened inside btcd rather than via a TLV +// pre-validator. This target asserts the no-panic property: any successful +// decode must re-encode without error, but a strict byte round-trip is not +// required because the canonical serialization may differ from a hostile +// non-canonical encoding. +func FuzzDecodeSig(f *testing.F) { + if b := fuzzSeedSig(); b != nil { + f.Add(b) + } + + f.Add([]byte{}) + f.Add([]byte{0x00}) + + // A transaction header declaring a near-2^64 input count: the canonical + // unbounded-allocation probe for the wire decoder. btcd must reject the + // count before allocating rather than crash. + f.Add([]byte{ + 0x01, 0x00, 0x00, 0x00, // version + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // varint + }) + + f.Fuzz(func(t *testing.T, data []byte) { + // The primary invariant: decoding hostile bytes must return an + // error, never panic or exhaust memory. + got, err := DecodeSig(data) + if err != nil { + return + } + + // Anything that decoded must re-encode without error. We do not + // assert byte equality because a non-canonical witness/legacy + // encoding can decode and re-serialize to canonical form. + if _, err := got.Encode(); err != nil { + t.Fatalf("re-encode decoded signature: %v", err) + } + }) +} + +// fuzzSeedSig builds a valid full-format signature payload to seed the corpus. +// Errors yield a nil return so seed-build failure cannot fail target setup. +func fuzzSeedSig() []byte { + tx := wire.NewMsgTx(2) + tx.AddTxIn(&wire.TxIn{ + PreviousOutPoint: wire.OutPoint{Index: 0}, + Witness: wire.TxWitness{{0x01, 0x02}}, + }) + tx.AddTxOut(&wire.TxOut{ + Value: 0, + PkScript: []byte{txscript.OP_RETURN}, + }) + + var b bytes.Buffer + if err := tx.Serialize(&b); err != nil { + return nil + } + + return b.Bytes() +} diff --git a/lib/recovery/proof_codec.go b/lib/recovery/proof_codec.go index 20905d7ed..7e3ed3985 100644 --- a/lib/recovery/proof_codec.go +++ b/lib/recovery/proof_codec.go @@ -91,6 +91,15 @@ func nodeDecoder(r io.Reader, val interface{}, _ *[8]byte, l uint64) error { ) } + // Bound the declared sub-stream length before allocating. l is the + // outer record length; an enclosing stream that framing-validated its + // records keeps l <= bytes present, but we re-check here so the make() + // is safe regardless of the caller. + if l > maxRecoveryMessageSize { + return fmt.Errorf("%w: node sub-stream %d bytes exceeds max %d", + ErrInvalidTLV, l, maxRecoveryMessageSize) + } + buf := make([]byte, l) if _, err := io.ReadFull(r, buf); err != nil { return err @@ -159,7 +168,15 @@ func decodeNodeStream(raw []byte) (*Node, error) { return nil, err } - parsed, err := stream.DecodeWithParsedTypes(bytes.NewReader(raw)) + // Pre-validate framing on the nested per-Node sub-stream too: the + // tx-bytes record decodes via DVarBytes, so a declared length larger + // than the sub-stream cannot be allowed to drive an unbounded make(). + reader, err := safeRecoveryTLVBytes(raw) + if err != nil { + return nil, fmt.Errorf("decode node: %w", err) + } + + parsed, err := stream.DecodeWithParsedTypes(reader) if err != nil { return nil, fmt.Errorf("decode node: %w", err) } @@ -243,7 +260,17 @@ func DecodeProof(raw []byte) (*Proof, error) { return nil, fmt.Errorf("create proof stream: %w", err) } - _, err = stream.DecodeWithParsedTypes(bytes.NewReader(raw)) + // Pre-validate the framing so a record declaring a length larger than + // the bytes present cannot drive an unbounded make() inside the tlv + // decoder (the DVarBytes-backed targetOutpoint and nodes records, plus + // the unknown-record discard buffer). Proof bytes are read back from + // durable storage on restart. + reader, err := safeRecoveryTLVBytes(raw) + if err != nil { + return nil, fmt.Errorf("decode proof: %w", err) + } + + _, err = stream.DecodeWithParsedTypes(reader) if err != nil { return nil, fmt.Errorf("decode proof: %w", err) } diff --git a/lib/recovery/proof_codec_fuzz_test.go b/lib/recovery/proof_codec_fuzz_test.go new file mode 100644 index 000000000..54d7ef5c7 --- /dev/null +++ b/lib/recovery/proof_codec_fuzz_test.go @@ -0,0 +1,92 @@ +package recovery + +import ( + "testing" + + "github.com/btcsuite/btcd/wire" +) + +// FuzzDecodeProof drives attacker-controlled bytes through the recovery proof +// decoder. Proof bytes are persisted durably and rebuilt on restart, so the +// decoder must never panic or allocate an unbounded buffer from a declared TLV +// length. The vulnerable surfaces are the outer DVarBytes records +// (targetOutpoint, nodes), the nested per-Node sub-stream decoder's +// make([]byte, l), and the wire.MsgTx deserializer inside each node. +// +// DecodeProof routes through NewProof, which enforces structural invariants +// (cycle-freedom, reachability, caps). A successful decode therefore yields a +// canonical proof that must re-encode and re-decode cleanly. +func FuzzDecodeProof(f *testing.F) { + if b := fuzzSeedProof(); b != nil { + f.Add(b) + } + + f.Add([]byte{}) + f.Add([]byte{0x00}) + + // targetOutpoint is TLV type 3 and decodes via DVarBytes. A tiny + // payload declaring a huge length is the canonical unbounded-make + // crasher. + f.Add([]byte{ + 0x03, 0xfe, 0xff, 0xff, 0xff, 0xff, + }) + + // The nodes record is TLV type 7, also DVarBytes-backed. + f.Add([]byte{ + 0x07, 0xfe, 0xff, 0xff, 0xff, 0xff, + }) + + // The canonical near-int64-max length crasher: type 0x0b followed by an + // 8-byte BigSize (0xff prefix) declaring 0x7fffffffffffffff value bytes + // in a 10-byte envelope. On the unhardened path this drives + // make([]byte, declaredLength) to "makeslice: cap out of range"; the + // framing pre-validator must reject it before any allocation. Pinned as + // a deterministic regression seed so the guard runs on every test pass. + f.Add([]byte{ + 0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + // The primary invariant: decoding hostile bytes must return an + // error, never panic or exhaust memory. + got, err := DecodeProof(data) + if err != nil { + return + } + + // A canonical proof must round-trip through encode/decode. + b2, err := EncodeProof(got) + if err != nil { + t.Fatalf("re-encode decoded proof: %v", err) + } + + if _, err := DecodeProof(b2); err != nil { + t.Fatalf("re-decode re-encoded proof: %v", err) + } + }) +} + +// fuzzSeedProof builds a minimal valid proof and returns its encoded bytes to +// seed the corpus. Errors yield a nil return so the seed is skipped rather than +// failing target setup. +func fuzzSeedProof() []byte { + tx := wire.NewMsgTx(wire.TxVersion) + tx.AddTxIn(&wire.TxIn{}) + tx.AddTxOut(&wire.TxOut{Value: 1000}) + + p, err := NewProof( + wire.OutPoint{Hash: tx.TxHash()}, + 10, + &Node{Kind: NodeKindArk, Tx: tx}, + ) + if err != nil { + return nil + } + + b, err := EncodeProof(p) + if err != nil { + return nil + } + + return b +} diff --git a/lib/recovery/state_codec.go b/lib/recovery/state_codec.go index e6be1f7d1..8b0a665a0 100644 --- a/lib/recovery/state_codec.go +++ b/lib/recovery/state_codec.go @@ -134,7 +134,17 @@ func DecodeSessionState(raw []byte) (*SessionState, error) { return nil, fmt.Errorf("create session state stream: %w", err) } - parsed, err := stream.DecodeWithParsedTypes(bytes.NewReader(raw)) + // Pre-validate the framing so a record declaring a length larger than + // the bytes present cannot drive an unbounded make() inside the tlv + // decoder (the DVarBytes-backed txStates, confirmHeights, failedTxid, + // and lastError records, plus the unknown-record discard buffer). + // Session state is read back from durable storage on restart. + reader, err := safeRecoveryTLVBytes(raw) + if err != nil { + return nil, fmt.Errorf("decode session state: %w", err) + } + + parsed, err := stream.DecodeWithParsedTypes(reader) if err != nil { return nil, fmt.Errorf("decode session state: %w", err) } diff --git a/lib/recovery/state_codec_fuzz_test.go b/lib/recovery/state_codec_fuzz_test.go new file mode 100644 index 000000000..74ac87316 --- /dev/null +++ b/lib/recovery/state_codec_fuzz_test.go @@ -0,0 +1,93 @@ +package recovery + +import ( + "testing" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/lightningnetwork/lnd/fn/v2" +) + +// FuzzDecodeSessionState drives attacker-controlled bytes through the durable +// session-state decoder. SessionState persists across restarts, so a corrupt +// or hostile on-disk blob must decode to an error, never panic or allocate an +// unbounded buffer from a declared TLV length. The vulnerable surfaces are the +// outer DVarBytes records (txStates, confirmHeights, failedTxid, lastError) +// and the unknown-record discard path. +// +// Encode is deterministic and canonical, so any successful decode must +// round-trip through encode/decode. +func FuzzDecodeSessionState(f *testing.F) { + if b := fuzzSeedSessionState(); b != nil { + f.Add(b) + } + + f.Add([]byte{}) + f.Add([]byte{0x00}) + + // txStates is TLV type 3 and decodes via DVarBytes. A tiny payload + // declaring a huge length is the canonical unbounded-make crasher. + f.Add([]byte{ + 0x03, 0xfe, 0xff, 0xff, 0xff, 0xff, + }) + + // lastError is TLV type 9, also DVarBytes-backed and unbounded. + f.Add([]byte{ + 0x09, 0xfe, 0xff, 0xff, 0xff, 0xff, + }) + + // The canonical near-int64-max length crasher: type 0x0b followed by an + // 8-byte BigSize (0xff prefix) declaring 0x7fffffffffffffff value bytes + // in a 10-byte envelope. On the unhardened path this drives + // make([]byte, declaredLength) to "makeslice: cap out of range"; the + // framing pre-validator must reject it before any allocation. Pinned as + // a deterministic regression seed so the guard runs on every test pass. + f.Add([]byte{ + 0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + // The primary invariant: decoding hostile bytes must return an + // error, never panic or exhaust memory. + got, err := DecodeSessionState(data) + if err != nil { + return + } + + // A canonical state must round-trip through encode/decode. + b2, err := EncodeSessionState(got) + if err != nil { + t.Fatalf("re-encode decoded state: %v", err) + } + + if _, err := DecodeSessionState(b2); err != nil { + t.Fatalf("re-decode re-encoded state: %v", err) + } + }) +} + +// fuzzSeedSessionState builds a representative state and returns its encoded +// bytes to seed the corpus. +func fuzzSeedSessionState() []byte { + var h1, h2 chainhash.Hash + h1[0] = 1 + h2[0] = 2 + + state := &SessionState{ + TxStates: map[chainhash.Hash]TxState{ + h1: TxStateConfirmed, + h2: TxStatePending, + }, + ConfirmHeights: map[chainhash.Hash]int32{ + h1: 123, + }, + FailedTxid: fn.Some(h2), + LastError: "package rejected", + } + + b, err := EncodeSessionState(state) + if err != nil { + return nil + } + + return b +} diff --git a/lib/recovery/testdata/fuzz/FuzzDecodeProof/6dd7e93fb2c59c26 b/lib/recovery/testdata/fuzz/FuzzDecodeProof/6dd7e93fb2c59c26 new file mode 100644 index 000000000..0b6a11e4d --- /dev/null +++ b/lib/recovery/testdata/fuzz/FuzzDecodeProof/6dd7e93fb2c59c26 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("0\xff00000000") diff --git a/lib/recovery/testdata/fuzz/FuzzDecodeSessionState/6dd7e93fb2c59c26 b/lib/recovery/testdata/fuzz/FuzzDecodeSessionState/6dd7e93fb2c59c26 new file mode 100644 index 000000000..0b6a11e4d --- /dev/null +++ b/lib/recovery/testdata/fuzz/FuzzDecodeSessionState/6dd7e93fb2c59c26 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("0\xff00000000") diff --git a/lib/recovery/tlv_bounds.go b/lib/recovery/tlv_bounds.go new file mode 100644 index 000000000..057b5f844 --- /dev/null +++ b/lib/recovery/tlv_bounds.go @@ -0,0 +1,99 @@ +package recovery + +import ( + "bytes" + "errors" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// ErrInvalidTLV is the sentinel wrapped by the bounded TLV decode helpers so +// callers can classify a malformed or oversized payload as an external decode +// failure rather than an internal bug. +var ErrInvalidTLV = errors.New("invalid tlv payload") + +// maxRecoveryMessageSize bounds the total wire size of an untrusted recovery +// TLV payload before any individual record length is honored. A proof can +// embed up to MaxProofNodes (100_000) recovery transactions, each carrying a +// serialized wire.MsgTx, so the cap is generous at 64 MiB while still +// rejecting the unbounded-allocation DoS where a tiny payload declares a +// multi-gigabyte (or near-2^64) record length. +// +// Recovery proofs and session state persist durably and are rebuilt on +// restart, so the bound is load-bearing on the replay path: a corrupt or +// hostile on-disk blob must decode to an error rather than crash the daemon at +// boot. +const maxRecoveryMessageSize = 64 << 20 + +// safeRecoveryTLVBytes validates an in-memory TLV blob and returns a reader +// positioned at its start. It pre-validates the (type, length) framing so the +// downstream tlv.Stream decoders can never be handed a record length larger +// than the bytes physically present. +// +// This is required because the tlv library sizes its value buffers with +// make([]byte, declaredLength) BEFORE reading any value bytes: stream.go's +// unknown-record path does bytes.NewBuffer(make([]byte, 0, length)) and +// primitive.go's DVarBytes does make([]byte, l). Neither bounds length against +// the input on the non-P2P DecodeWithParsedTypes path. A producer-declared +// length near 2^64 panics the decoder with "makeslice: cap out of range", and +// a multi-gigabyte length drives an OOM. By rejecting any record whose declared +// length exceeds the remaining buffer (a record can never legitimately carry +// more bytes than the message contains), we cap every allocation at the +// message size, which is itself capped at maxRecoveryMessageSize. +func safeRecoveryTLVBytes(raw []byte) (*bytes.Reader, error) { + if len(raw) > maxRecoveryMessageSize { + return nil, fmt.Errorf("%w: message %d bytes exceeds max %d", + ErrInvalidTLV, len(raw), maxRecoveryMessageSize) + } + + if err := validateTLVRecordLengths(raw); err != nil { + return nil, err + } + + return bytes.NewReader(raw), nil +} + +// validateTLVRecordLengths walks the (type, length, value) framing of a +// buffered TLV stream and rejects any record whose declared length exceeds the +// bytes remaining in the buffer. It deliberately does not interpret record +// contents; it only ensures the framing cannot drive an over-sized make() in +// the real decoder before any value bytes are read. +func validateTLVRecordLengths(buf []byte) error { + var scratch [8]byte + br := bytes.NewReader(buf) + + for br.Len() > 0 { + // The type varint read only reaches EOF when br.Len() == 0 + // above; any failure here is a malformed stream. + if _, err := tlv.ReadVarInt(br, &scratch); err != nil { + return fmt.Errorf("%w: read tlv type: %v", + ErrInvalidTLV, err) + } + + length, err := tlv.ReadVarInt(br, &scratch) + if err != nil { + return fmt.Errorf("%w: read tlv length: %v", + ErrInvalidTLV, err) + } + + // A record can never carry more value bytes than remain in the + // buffer; reject before the decoder allocates make([]byte, + // length). + if length > uint64(br.Len()) { + return fmt.Errorf("%w: tlv record length %d exceeds %d "+ + "remaining bytes", ErrInvalidTLV, length, + br.Len()) + } + + if _, err := br.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return fmt.Errorf("%w: skip tlv value: %v", + ErrInvalidTLV, err) + } + } + + return nil +} diff --git a/lib/tx/checkpoint/safetlv.go b/lib/tx/checkpoint/safetlv.go new file mode 100644 index 000000000..87e9bc990 --- /dev/null +++ b/lib/tx/checkpoint/safetlv.go @@ -0,0 +1,71 @@ +package checkpoint + +import ( + "bytes" + "errors" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// ErrTLVRecordTooLarge signals that a TLV record declared a length larger than +// the bytes physically present in the input. The lnd tlv stream decoder's +// non-P2P path sizes make([]byte, length) (both for known DVarBytes records +// and the unknown-record discard buffer) directly from this attacker-controlled +// length without bounding it against the reader, so a tiny payload declaring a +// huge length panics (makeslice) or OOMs. We pre-validate framing to fail +// closed instead. +var ErrTLVRecordTooLarge = errors.New("tlv record length exceeds input") + +// safeTLVReader validates the (type, length) framing of an in-memory TLV blob +// against the bytes physically present, then returns a reader over the same +// bytes for the real decode. +// +// The check is intentionally cheap: every record costs at least its declared +// length in payload bytes, so a record claiming more bytes than remain in the +// buffer is definitionally malformed. Rejecting it up front means the +// subsequent tlv.Stream.Decode can never reach the unbounded make([]byte, +// length) site with a length larger than the input. +func safeTLVReader(raw []byte) (*bytes.Reader, error) { + if err := validateTLVFraming(raw); err != nil { + return nil, err + } + + return bytes.NewReader(raw), nil +} + +// validateTLVFraming walks the record framing of an in-memory TLV blob and +// rejects any record whose declared length is larger than the remaining bytes. +func validateTLVFraming(raw []byte) error { + var scratch [8]byte + + reader := bytes.NewReader(raw) + for reader.Len() > 0 { + // The type varint is part of framing, not payload; an error + // here is a normal short read that the real decoder will also + // surface, so we stop validating and let it produce the + // canonical error. + if _, err := tlv.ReadVarInt(reader, &scratch); err != nil { + return nil + } + + length, err := tlv.ReadVarInt(reader, &scratch) + if err != nil { + return nil + } + + if length > uint64(reader.Len()) { + return fmt.Errorf("%w: declared %d, %d remaining", + ErrTLVRecordTooLarge, length, reader.Len()) + } + + if _, err := reader.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return nil + } + } + + return nil +} diff --git a/lib/tx/checkpoint/taptree.go b/lib/tx/checkpoint/taptree.go index cd23cb496..7857d8a4c 100644 --- a/lib/tx/checkpoint/taptree.go +++ b/lib/tx/checkpoint/taptree.go @@ -3,6 +3,7 @@ package checkpoint import ( "bytes" "errors" + "fmt" "io" "github.com/btcsuite/btcd/txscript" @@ -86,7 +87,16 @@ func DecodeTapTree(data []byte) ([][]byte, error) { return nil, err } - _, err = tlvStream.DecodeWithParsedTypes(bytes.NewReader(data)) + // Validate the outer record framing against the bytes physically + // present before decoding. The lnd tlv non-P2P decode path sizes + // allocations from the declared record length without bounding it, so + // an attacker-controlled length could otherwise panic or OOM here. + safeReader, err := safeTLVReader(data) + if err != nil { + return nil, err + } + + _, err = tlvStream.DecodeWithParsedTypes(safeReader) if err != nil { return nil, err } @@ -162,9 +172,31 @@ func leavesDecoder(r io.Reader, val interface{}, buf *[8]byte, l uint64) error { return err } - innerInnerTlvReader := io.LimitedReader{ - R: &innerTlvReader, - N: int64(blobSize), + // Bound the declared per-leaf blob length against the + // bytes still available in the outer record. A leaf + // claiming more bytes than remain is malformed; without + // this the inner stream decode below would size a + // make([]byte, length) from this untrusted value and + // could panic or OOM. + if blobSize > uint64(innerTlvReader.N) { + return fmt.Errorf("%w: leaf blob declared %d, "+ + "%d remaining", ErrTLVRecordTooLarge, + blobSize, innerTlvReader.N) + } + + // Buffer the exact leaf blob and validate its inner + // record framing before decoding, so the inner DVarBytes + // and unknown-record paths cannot over-allocate either. + leafBlob := make([]byte, blobSize) + if _, err := io.ReadFull( + &innerTlvReader, leafBlob, + ); err != nil { + return err + } + + leafReader, err := safeTLVReader(leafBlob) + if err != nil { + return err } var ( @@ -184,7 +216,7 @@ func leavesDecoder(r io.Reader, val interface{}, buf *[8]byte, l uint64) error { } parsedTypes, err := tlvStream.DecodeWithParsedTypes( - &innerInnerTlvReader, + leafReader, ) if err != nil { return err diff --git a/lib/tx/checkpoint/taptree_fuzz_test.go b/lib/tx/checkpoint/taptree_fuzz_test.go new file mode 100644 index 000000000..e837495c1 --- /dev/null +++ b/lib/tx/checkpoint/taptree_fuzz_test.go @@ -0,0 +1,59 @@ +package checkpoint + +import ( + "testing" +) + +// FuzzDecodeTapTree drives DecodeTapTree with attacker-controlled bytes. The +// tap tree blob is persisted as PSBT sidecar metadata and flows from +// operator-sourced OOR artifacts, so it crosses the trust boundary. The leaf +// stream nests a varint-length-prefixed inner TLV per leaf, and each leaf +// script decodes through tlv DVarBytes (an unbounded make([]byte, length)), +// making this a prime unbounded-allocation target. +func FuzzDecodeTapTree(f *testing.F) { + if seed, err := EncodeTapTree([][]byte{ + {0x51, 0x51, 0x51}, + {0x6a}, + {0x00, 0x01, 0x02, 0x03}, + }); err == nil { + f.Add(seed) + } + f.Add([]byte{}) + + // Canonical malicious payload: TLV type 11, then a BigSize-encoded + // length of math.MaxInt64. Without the safeTLVReader framing guard the + // decoder would size make([]byte, declaredLength) from this value and + // panic (makeslice) or OOM. Seeding it pins the guard's rejection path. + f.Add([]byte{ + 0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + // A malformed tap tree must error rather than panic/OOM. + leaves, err := DecodeTapTree(data) + if err != nil { + return + } + + // A clean decode must round-trip: re-encoding the recovered + // scripts and decoding again must yield the same scripts. We do + // not assert byte-equality against the input because DecodeTapTree + // is intentionally lenient (drops leaf versions, ignores unknown + // records), so the canonical re-encoding can differ from a + // hand-crafted input. + reencoded, err := EncodeTapTree(leaves) + if err != nil { + t.Fatalf("re-encode: %v", err) + } + + redecoded, err := DecodeTapTree(reencoded) + if err != nil { + t.Fatalf("re-decode: %v", err) + } + + if len(redecoded) != len(leaves) { + t.Fatalf("leaf count drift: got %d want %d", + len(redecoded), len(leaves)) + } + }) +} diff --git a/lib/tx/checkpoint/testdata/fuzz/FuzzDecodeTapTree/6dd7e93fb2c59c26 b/lib/tx/checkpoint/testdata/fuzz/FuzzDecodeTapTree/6dd7e93fb2c59c26 new file mode 100644 index 000000000..0b6a11e4d --- /dev/null +++ b/lib/tx/checkpoint/testdata/fuzz/FuzzDecodeTapTree/6dd7e93fb2c59c26 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("0\xff00000000") diff --git a/lib/tx/oor/package.go b/lib/tx/oor/package.go index ff35af372..fe09c43c5 100644 --- a/lib/tx/oor/package.go +++ b/lib/tx/oor/package.go @@ -164,7 +164,15 @@ func UnmarshalSubmitPackage(b []byte) (*SubmitPackage, error) { return nil, err } - reader := bytes.NewReader(b) + // Validate the record framing against the bytes physically present + // before decoding. The lnd tlv non-P2P decode path sizes allocations + // from the declared record length without bounding it, so an + // attacker-controlled length on this RPC/durable path could otherwise + // panic or OOM. + reader, err := safeTLVReader(b) + if err != nil { + return nil, err + } if _, err := stream.DecodeWithParsedTypes(reader); err != nil { return nil, err } @@ -242,6 +250,16 @@ func decodeBlobList(raw []byte) ([][]byte, error) { return nil, err } + // Bound the declared count against the bytes physically present + // before allocating. Each blob is encoded as at least a one-byte + // length prefix, so a count larger than the remaining bytes is + // definitionally a lie and would otherwise drive an unbounded (or + // panicking) make() from attacker-controlled input. + if count > uint64(reader.Len()) { + return nil, fmt.Errorf("blob count %d exceeds %d remaining "+ + "bytes", count, reader.Len()) + } + blobs := make([][]byte, 0, count) for i := uint64(0); i < count; i++ { size, err := tlv.ReadVarInt(reader, &scratch) @@ -249,6 +267,14 @@ func decodeBlobList(raw []byte) ([][]byte, error) { return nil, err } + // Likewise bound each blob length against the bytes still + // available so a huge declared size cannot allocate gigabytes + // before io.ReadFull discovers the truncation. + if size > uint64(reader.Len()) { + return nil, fmt.Errorf("blob %d size %d exceeds %d "+ + "remaining bytes", i, size, reader.Len()) + } + blob := make([]byte, size) if _, err := io.ReadFull(reader, blob); err != nil { return nil, err diff --git a/lib/tx/oor/package_fuzz_test.go b/lib/tx/oor/package_fuzz_test.go new file mode 100644 index 000000000..f01d6f639 --- /dev/null +++ b/lib/tx/oor/package_fuzz_test.go @@ -0,0 +1,131 @@ +package oor + +import ( + "bytes" + "testing" + + "github.com/btcsuite/btcd/btcutil/psbt" + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/darepo-client/lib/arkscript" +) + +// fuzzSeedSubmitPackage builds a representative, valid submit package whose +// marshaled bytes serve as a structured seed for the fuzzer. Mirroring the +// shape exercised in package_test.go gives the fuzzer a real TLV framing to +// mutate from rather than purely random noise. +func fuzzSeedSubmitPackage() ([]byte, bool) { + checkpointTx := wire.NewMsgTx(3) + checkpointTx.AddTxIn(&wire.TxIn{}) + checkpointTx.AddTxOut(&wire.TxOut{Value: 5, PkScript: []byte{0x51}}) + checkpointTx.AddTxOut(arkscript.AnchorOutput()) + + checkpointPSBT, err := psbt.NewFromUnsignedTx(checkpointTx) + if err != nil { + return nil, false + } + + arkTx := wire.NewMsgTx(3) + arkTx.AddTxIn(&wire.TxIn{ + PreviousOutPoint: wire.OutPoint{ + Hash: checkpointTx.TxHash(), + Index: 0, + }, + }) + arkTx.AddTxOut(&wire.TxOut{Value: 5, PkScript: []byte{0x51}}) + arkTx.AddTxOut(arkscript.AnchorOutput()) + + arkPSBT, err := psbt.NewFromUnsignedTx(arkTx) + if err != nil { + return nil, false + } + + pkg := &SubmitPackage{ + ArkPSBT: arkPSBT, + CheckpointPSBTs: []*psbt.Packet{checkpointPSBT}, + } + + raw, err := MarshalSubmitPackage(pkg) + if err != nil { + return nil, false + } + + return raw, true +} + +// FuzzUnmarshalSubmitPackage drives UnmarshalSubmitPackage with attacker +// controlled bytes. These bytes cross the OOR submit RPC trust boundary and +// are persisted durably, so a decode must never panic, OOM, or read OOB. The +// blob-list framing (count + per-blob length prefixes) is the prime suspect: +// both the outer count and each inner length feed make() calls. +func FuzzUnmarshalSubmitPackage(f *testing.F) { + if seed, ok := fuzzSeedSubmitPackage(); ok { + f.Add(seed) + } + f.Add([]byte{}) + + // Canonical malicious payload: TLV type 11, then a BigSize-encoded + // length of math.MaxInt64. Without the safeTLVReader framing guard the + // decoder would size make([]byte, declaredLength) from this value and + // panic (makeslice) or OOM. Seeding it pins the guard's rejection path. + f.Add([]byte{ + 0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + // A successful decode is not required; we only require that a + // malformed payload fails cleanly instead of crashing the + // process. + pkg, err := UnmarshalSubmitPackage(data) + if err != nil { + return + } + + // On a clean decode the re-marshal must succeed and round-trip + // back to an equivalent structure, proving the decoder produced + // a self-consistent value. + out, err := MarshalSubmitPackage(pkg) + if err != nil { + t.Fatalf("re-marshal: %v", err) + } + + if _, err := UnmarshalSubmitPackage(out); err != nil { + t.Fatalf("re-unmarshal: %v", err) + } + }) +} + +// FuzzDecodeBlobList targets the internal blob-list codec directly so the +// fuzzer can reach the count/length make() sites without first satisfying the +// outer package TLV framing. +func FuzzDecodeBlobList(f *testing.F) { + if seed, err := encodeBlobList([][]byte{ + {0x01, 0x02}, {}, {0xff}, + }); err == nil { + f.Add(seed) + } + f.Add([]byte{}) + + // Canonical malicious payload: a BigSize-encoded count of math.MaxInt64. + // decodeBlobList must reject this against the remaining-bytes bound + // before reaching make([][]byte, count). + f.Add([]byte{ + 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + blobs, err := decodeBlobList(data) + if err != nil { + return + } + + out, err := encodeBlobList(blobs) + if err != nil { + t.Fatalf("re-encode: %v", err) + } + + if !bytes.Equal(out, data) { + t.Fatalf("blob list not byte-stable:\n got %x\nwant %x", + out, data) + } + }) +} diff --git a/lib/tx/oor/safetlv.go b/lib/tx/oor/safetlv.go new file mode 100644 index 000000000..60d7f3e52 --- /dev/null +++ b/lib/tx/oor/safetlv.go @@ -0,0 +1,67 @@ +package oor + +import ( + "bytes" + "errors" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// ErrTLVRecordTooLarge signals that a TLV record declared a length larger than +// the bytes physically present in the input. The lnd tlv stream decoder's +// non-P2P path sizes make([]byte, length) (both for known DVarBytes records +// and the unknown-record discard buffer) directly from this attacker-controlled +// length without bounding it against the reader, so a tiny payload declaring a +// huge length panics (makeslice) or OOMs. We pre-validate framing to fail +// closed instead. OOR submit/finalize packages cross the RPC trust boundary +// and are persisted durably, so this guard sits on a hostile path. +var ErrTLVRecordTooLarge = errors.New("tlv record length exceeds input") + +// safeTLVReader validates the (type, length) framing of an in-memory TLV blob +// against the bytes physically present, then returns a reader over the same +// bytes for the real decode. Rejecting an over-long record up front means the +// subsequent tlv.Stream.Decode can never reach the unbounded make([]byte, +// length) site with a length larger than the input. +func safeTLVReader(raw []byte) (*bytes.Reader, error) { + if err := validateTLVFraming(raw); err != nil { + return nil, err + } + + return bytes.NewReader(raw), nil +} + +// validateTLVFraming walks the record framing of an in-memory TLV blob and +// rejects any record whose declared length is larger than the remaining bytes. +func validateTLVFraming(raw []byte) error { + var scratch [8]byte + + reader := bytes.NewReader(raw) + for reader.Len() > 0 { + // A short read on the framing varints is a normal truncation + // the real decoder will also surface, so we stop validating and + // defer to its canonical error. + if _, err := tlv.ReadVarInt(reader, &scratch); err != nil { + return nil + } + + length, err := tlv.ReadVarInt(reader, &scratch) + if err != nil { + return nil + } + + if length > uint64(reader.Len()) { + return fmt.Errorf("%w: declared %d, %d remaining", + ErrTLVRecordTooLarge, length, reader.Len()) + } + + if _, err := reader.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return nil + } + } + + return nil +} diff --git a/lib/tx/oor/testdata/fuzz/FuzzDecodeBlobList/b6db8bb3e4863f3b b/lib/tx/oor/testdata/fuzz/FuzzDecodeBlobList/b6db8bb3e4863f3b new file mode 100644 index 000000000..1b6494074 --- /dev/null +++ b/lib/tx/oor/testdata/fuzz/FuzzDecodeBlobList/b6db8bb3e4863f3b @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("\xff00000000") diff --git a/lib/types/codec.go b/lib/types/codec.go index 0431a11c1..d02ba15f0 100644 --- a/lib/types/codec.go +++ b/lib/types/codec.go @@ -254,7 +254,16 @@ func DecodeJoinRoundAuthMessage(raw []byte) (*JoinRoundRequest, error) { err) } - reader := bytes.NewReader(raw) + // Pre-validate the framing so a record declaring a length larger than + // the bytes present cannot drive an unbounded make() inside the tlv + // decoder. These bytes are the attacker-controlled, BIP-322-signed join + // intent crossing the client/server trust boundary. + reader, err := safeTypesTLVBytes(raw) + if err != nil { + return nil, fmt.Errorf("decode join auth message envelope: %w", + err) + } + parsedTypes, err := stream.DecodeWithParsedTypes(reader) if err != nil { return nil, fmt.Errorf("decode join auth message envelope: %w", @@ -411,7 +420,11 @@ func decodeJoinAuthBoardingRequest(raw []byte) (*BoardingRequest, error) { return nil, fmt.Errorf("create boarding decode stream: %w", err) } - reader := bytes.NewReader(raw) + reader, err := safeTypesTLVBytes(raw) + if err != nil { + return nil, fmt.Errorf("decode boarding request: %w", err) + } + parsedTypes, err := stream.DecodeWithParsedTypes(reader) if err != nil { return nil, fmt.Errorf("decode boarding request: %w", err) @@ -512,7 +525,11 @@ func decodeJoinAuthVTXORequest(raw []byte) (*VTXORequest, error) { return nil, fmt.Errorf("create vtxo decode stream: %w", err) } - reader := bytes.NewReader(raw) + reader, err := safeTypesTLVBytes(raw) + if err != nil { + return nil, fmt.Errorf("decode vtxo request: %w", err) + } + parsedTypes, err := stream.DecodeWithParsedTypes(reader) if err != nil { return nil, fmt.Errorf("decode vtxo request: %w", err) @@ -617,7 +634,11 @@ func decodeJoinAuthForfeitRequest(raw []byte) (*ForfeitRequest, error) { return nil, fmt.Errorf("create forfeit decode stream: %w", err) } - reader := bytes.NewReader(raw) + reader, err := safeTypesTLVBytes(raw) + if err != nil { + return nil, fmt.Errorf("decode forfeit request: %w", err) + } + parsedTypes, err := stream.DecodeWithParsedTypes(reader) if err != nil { return nil, fmt.Errorf("decode forfeit request: %w", err) @@ -699,7 +720,11 @@ func decodeJoinAuthLeaveRequest(raw []byte) (*LeaveRequest, error) { return nil, fmt.Errorf("create leave decode stream: %w", err) } - reader := bytes.NewReader(raw) + reader, err := safeTypesTLVBytes(raw) + if err != nil { + return nil, fmt.Errorf("decode leave request: %w", err) + } + parsedTypes, err := stream.DecodeWithParsedTypes(reader) if err != nil { return nil, fmt.Errorf("decode leave request: %w", err) diff --git a/lib/types/codec_fuzz_test.go b/lib/types/codec_fuzz_test.go new file mode 100644 index 000000000..381c50c67 --- /dev/null +++ b/lib/types/codec_fuzz_test.go @@ -0,0 +1,86 @@ +package types + +import ( + "testing" + + "github.com/btcsuite/btcd/btcec/v2" +) + +// FuzzDecodeJoinRoundAuthMessage drives attacker-controlled bytes through the +// join-round auth decoder. These bytes cross the client/server trust boundary +// (a client supplies them as the BIP-322-signed join intent), so the decoder +// must never panic, OOM, or allocate an unbounded buffer from a declared TLV +// length. The decoder enforces strict framing, so any successfully decoded +// value must re-encode and re-decode cleanly. +func FuzzDecodeJoinRoundAuthMessage(f *testing.F) { + // Seed with a valid canonical message so the fuzzer starts from a + // structurally sound corpus and mutates outward from there. + if b := fuzzSeedJoinRoundAuthMessage(); b != nil { + f.Add(b) + } + + // Degenerate seeds that probe the early-return and framing guards. + f.Add([]byte{}) + f.Add([]byte{0x00}) + f.Add([]byte{0x01}) + + // A record that declares a huge length but carries no payload. This is + // the canonical unbounded-make crasher: type 2, length 0xffffffff. + f.Add([]byte{ + 0x02, 0xfe, 0xff, 0xff, 0xff, 0xff, + }) + + // The canonical near-int64-max length crasher: type 0x0b followed by an + // 8-byte BigSize (0xff prefix) declaring 0x7fffffffffffffff value bytes + // in a 10-byte envelope. On the unhardened path this drives + // make([]byte, declaredLength) to "makeslice: cap out of range"; the + // framing pre-validator must reject it before any allocation. Pinned as + // a deterministic regression seed so the guard is exercised on every + // `go test` run, not just under active fuzzing. + f.Add([]byte{ + 0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + // The primary invariant: decoding hostile bytes must return an + // error, never panic or exhaust memory. + got, err := DecodeJoinRoundAuthMessage(data) + if err != nil { + return + } + + // A value that decoded cleanly must round-trip. Re-encoding then + // re-decoding catches asymmetries where the decoder accepts + // something the encoder cannot reproduce. + b2, err := JoinRoundAuthMessage(got) + if err != nil { + t.Fatalf("re-encode decoded message: %v", err) + } + + if _, err := DecodeJoinRoundAuthMessage(b2); err != nil { + t.Fatalf("re-decode re-encoded message: %v", err) + } + }) +} + +// fuzzSeedJoinRoundAuthMessage builds a valid canonical join-auth payload to +// seed the corpus. It mirrors testJoinRoundAuthRequest but returns only the +// bytes, swallowing errors so a seed-build failure cannot fail the fuzz target +// setup; a nil return simply means the seed is skipped. +func fuzzSeedJoinRoundAuthMessage() []byte { + priv, err := btcec.NewPrivateKey() + if err != nil { + return nil + } + + req := &JoinRoundRequest{ + Identifier: priv.PubKey(), + } + + b, err := JoinRoundAuthMessage(req) + if err != nil { + return nil + } + + return b +} diff --git a/lib/types/proof_codec.go b/lib/types/proof_codec.go index 779822e89..e40688234 100644 --- a/lib/types/proof_codec.go +++ b/lib/types/proof_codec.go @@ -299,7 +299,16 @@ func DeserializeTxProof(data []byte) (*proof.TxProof, error) { return nil, fmt.Errorf("create TLV stream: %w", err) } - reader := bytes.NewReader(data) + // Pre-validate the framing so a record declaring a length larger than + // the bytes present cannot drive an unbounded make() inside the tlv + // decoder (notably the variable-length MerkleRoot record and the + // unknown-record discard buffer). TxProof bytes arrive as an untrusted + // boarding SPV proof and also persist durably. + reader, err := safeTypesTLVBytes(data) + if err != nil { + return nil, fmt.Errorf("decode TxProof: %w", err) + } + if err := stream.Decode(reader); err != nil { return nil, fmt.Errorf("decode TxProof: %w", err) } diff --git a/lib/types/proof_codec_fuzz_test.go b/lib/types/proof_codec_fuzz_test.go new file mode 100644 index 000000000..9133cfd8f --- /dev/null +++ b/lib/types/proof_codec_fuzz_test.go @@ -0,0 +1,63 @@ +package types + +import ( + "testing" +) + +// FuzzDeserializeTxProof drives attacker-controlled bytes through the TxProof +// TLV decoder. TxProof bytes cross the trust boundary as a boarding SPV proof +// the server verifies and also persist durably, so the decoder must never +// panic or allocate an unbounded buffer from a declared TLV length (notably +// the variable-length MerkleRoot record and the unknown-record discard path). +// +// The encode path is not byte-for-byte symmetric for every decodable value +// (a decoded proof may omit optional records the encoder always writes), so we +// assert the no-panic property and that any successful decode can be +// re-serialized without error rather than a strict byte round-trip. +func FuzzDeserializeTxProof(f *testing.F) { + f.Add([]byte{}) + f.Add([]byte{0x00}) + + // MerkleRoot is TLV type 6 and decodes via DVarBytes. A tiny payload + // declaring a huge length is the canonical unbounded-make crasher. + f.Add([]byte{ + 0x06, 0xfe, 0xff, 0xff, 0xff, 0xff, + }) + + // The MsgTx record is type 0 and feeds wire.MsgTx.Deserialize, which + // reads its own attacker-controlled var-length prefixes. Probe it with + // a declared-large outer length and an empty body. + f.Add([]byte{ + 0x00, 0xfe, 0xff, 0xff, 0xff, 0xff, + }) + + // The canonical near-int64-max length crasher: type 0x0b followed by an + // 8-byte BigSize (0xff prefix) declaring 0x7fffffffffffffff value bytes + // in a 10-byte envelope. On the unhardened path this drives + // make([]byte, declaredLength) to "makeslice: cap out of range"; the + // framing pre-validator must reject it before any allocation. Pinned as + // a deterministic regression seed so the guard runs on every test pass. + f.Add([]byte{ + 0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + // The primary invariant: decoding hostile bytes must return an + // error, never panic or exhaust memory. + got, err := DeserializeTxProof(data) + if err != nil { + return + } + + // A nil proof (empty input) is a valid no-op decode result. + if got == nil { + return + } + + // Anything that decoded must re-serialize without error. We do + // not assert byte equality because optional records can differ. + if _, err := SerializeTxProof(got); err != nil { + t.Fatalf("re-serialize decoded proof: %v", err) + } + }) +} diff --git a/lib/types/testdata/fuzz/FuzzDecodeJoinRoundAuthMessage/6dd7e93fb2c59c26 b/lib/types/testdata/fuzz/FuzzDecodeJoinRoundAuthMessage/6dd7e93fb2c59c26 new file mode 100644 index 000000000..0b6a11e4d --- /dev/null +++ b/lib/types/testdata/fuzz/FuzzDecodeJoinRoundAuthMessage/6dd7e93fb2c59c26 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("0\xff00000000") diff --git a/lib/types/testdata/fuzz/FuzzDeserializeTxProof/e070db028c0c6ac0 b/lib/types/testdata/fuzz/FuzzDeserializeTxProof/e070db028c0c6ac0 new file mode 100644 index 000000000..6c7694ac6 --- /dev/null +++ b/lib/types/testdata/fuzz/FuzzDeserializeTxProof/e070db028c0c6ac0 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("\x06\xff00000000") diff --git a/lib/types/tlv_bounds.go b/lib/types/tlv_bounds.go new file mode 100644 index 000000000..eb307ceb9 --- /dev/null +++ b/lib/types/tlv_bounds.go @@ -0,0 +1,103 @@ +package types + +import ( + "bytes" + "errors" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// ErrInvalidTLV is the sentinel wrapped by the bounded TLV decode helpers so +// callers can classify a malformed or oversized payload as an external decode +// failure rather than an internal bug. +var ErrInvalidTLV = errors.New("invalid tlv payload") + +// maxTypesMessageSize bounds the total wire size of an untrusted types TLV +// payload before any individual record length is honored. The largest +// legitimate payload is a join-round auth message: up to +// joinRoundAuthMaxRequestCount (1024) entries per list at +// joinRoundAuthMaxBlobEntrySize (64 KiB) each, across four lists, plus the +// TxProof codec which embeds a full transaction and merkle proof. 16 MiB gives +// generous headroom for the largest legitimate body while still rejecting the +// unbounded-allocation DoS where a tiny payload declares a multi-gigabyte (or +// near-2^64) record length. +// +// These payloads cross the client/server trust boundary (join-auth is the +// BIP-322-signed join intent; TxProof is a boarding SPV proof the server +// verifies) and TxProof additionally persists durably, so the bound is +// load-bearing on both the live and replay path. +const maxTypesMessageSize = 16 << 20 + +// safeTypesTLVBytes validates an in-memory TLV blob and returns a reader +// positioned at its start. It pre-validates the (type, length) framing so the +// downstream tlv.Stream decoders can never be handed a record length larger +// than the bytes physically present. +// +// This is required because the tlv library sizes its value buffers with +// make([]byte, declaredLength) BEFORE reading any value bytes: stream.go's +// unknown-record path does bytes.NewBuffer(make([]byte, 0, length)) and +// primitive.go's DVarBytes does make([]byte, l). Neither bounds length against +// the input on the non-P2P Decode / DecodeWithParsedTypes path. A +// producer-declared length near 2^64 panics the decoder with "makeslice: cap +// out of range", and a multi-gigabyte length drives an OOM. By rejecting any +// record whose declared length exceeds the remaining buffer (a record can +// never legitimately carry more bytes than the message contains), we cap every +// allocation at the message size, which is itself capped at +// maxTypesMessageSize. +func safeTypesTLVBytes(raw []byte) (*bytes.Reader, error) { + if len(raw) > maxTypesMessageSize { + return nil, fmt.Errorf("%w: message %d bytes exceeds max %d", + ErrInvalidTLV, len(raw), maxTypesMessageSize) + } + + if err := validateTLVRecordLengths(raw); err != nil { + return nil, err + } + + return bytes.NewReader(raw), nil +} + +// validateTLVRecordLengths walks the (type, length, value) framing of a +// buffered TLV stream and rejects any record whose declared length exceeds the +// bytes remaining in the buffer. It deliberately does not interpret record +// contents; it only ensures the framing cannot drive an over-sized make() in +// the real decoder before any value bytes are read. +func validateTLVRecordLengths(buf []byte) error { + var scratch [8]byte + br := bytes.NewReader(buf) + + for br.Len() > 0 { + // The type varint read only reaches EOF when br.Len() == 0 + // above; any failure here is a malformed stream. + if _, err := tlv.ReadVarInt(br, &scratch); err != nil { + return fmt.Errorf("%w: read tlv type: %v", + ErrInvalidTLV, err) + } + + length, err := tlv.ReadVarInt(br, &scratch) + if err != nil { + return fmt.Errorf("%w: read tlv length: %v", + ErrInvalidTLV, err) + } + + // A record can never carry more value bytes than remain in the + // buffer; reject before the decoder allocates make([]byte, + // length). + if length > uint64(br.Len()) { + return fmt.Errorf("%w: tlv record length %d exceeds %d "+ + "remaining bytes", ErrInvalidTLV, length, + br.Len()) + } + + if _, err := br.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return fmt.Errorf("%w: skip tlv value: %v", + ErrInvalidTLV, err) + } + } + + return nil +} diff --git a/mailbox/conn/ack_state.go b/mailbox/conn/ack_state.go index 188f8fcb3..b3e3f2c46 100644 --- a/mailbox/conn/ack_state.go +++ b/mailbox/conn/ack_state.go @@ -122,7 +122,16 @@ func (s *AckState) Decode(r io.Reader) error { return err } - _, err = stream.DecodeWithParsedTypes(r) + // Bound the untrusted payload before decode: AckState is persisted + // in the durable checkpoint store and replayed from disk, so a + // crafted record length must not drive an unbounded make() in the + // tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + _, err = stream.DecodeWithParsedTypes(safe) return err } diff --git a/mailbox/conn/proto_record.go b/mailbox/conn/proto_record.go index d5427a907..1af61d08e 100644 --- a/mailbox/conn/proto_record.go +++ b/mailbox/conn/proto_record.go @@ -1,6 +1,7 @@ package conn import ( + "fmt" "io" "reflect" @@ -86,6 +87,18 @@ func wrappedProtoDecoder[T proto.Message]( ) } + // Bound the declared record length against a sane per-message cap + // before make([]byte, l). The tlv library does not cap the record + // length on the non-p2p decode path, so a crafted length near 2^64 + // would otherwise panic with "makeslice: len out of range" or OOM. + // Callers wrap their Decode entry point in safeTLVReader, which + // already rejects a length larger than the buffered payload; this + // guard is defense-in-depth for any direct use of the record. + if l > maxConnMessageSize { + return fmt.Errorf("%w: wrapped proto length %d exceeds max %d", + ErrInvalidTLV, l, maxConnMessageSize) + } + data := make([]byte, l) if _, err := io.ReadFull(r, data); err != nil { return err diff --git a/mailbox/conn/safe_tlv.go b/mailbox/conn/safe_tlv.go new file mode 100644 index 000000000..e4b99bed7 --- /dev/null +++ b/mailbox/conn/safe_tlv.go @@ -0,0 +1,106 @@ +package conn + +import ( + "bytes" + "errors" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// ErrInvalidTLV is the sentinel wrapped by the bounded TLV decode +// helpers so callers can classify a malformed/oversized payload as an +// external decode failure rather than an internal bug. +var ErrInvalidTLV = errors.New("invalid tlv payload") + +// maxConnMessageSize bounds the total wire size of an untrusted +// mailbox/conn TLV payload before any individual record length is +// honored. AckState is four uint64 cursors; the WrappedProto-bearing +// serverconn messages embed envelopes/Any payloads that are +// themselves bounded by the mailbox transport. 4 MiB gives generous +// headroom for the largest legitimate proto body while still +// rejecting the unbounded-allocation DoS where a tiny payload +// declares a multi-gigabyte (or near-2^64) record length. +const maxConnMessageSize = 4 << 20 + +// safeTLVReader reads an untrusted TLV payload into a size-capped +// buffer and pre-validates the (type, length, value) framing so the +// downstream tlv.Stream.DecodeWithParsedTypes can never be handed a +// record length larger than the bytes physically present. +// +// This is load-bearing because the tlv library sizes its value +// buffers with make([]byte, declaredLength) BEFORE reading any value +// bytes: stream.go does bytes.NewBuffer(make([]byte, 0, length)) for +// unknown records, and primitive.go's DVarBytes does +// make([]byte, l) for known []byte fields. A producer-declared length +// near 2^64 panics the decoder with "makeslice: cap out of range", +// and a multi-gigabyte length drives an OOM. Both are reachable from a +// few attacker-controlled bytes that cross the client/server trust +// boundary and persist in the durable mailbox (replayed from disk +// across upgrades). By rejecting any record whose declared length +// exceeds the remaining buffer we cap every allocation at the message +// size, which is itself capped at maxConnMessageSize. +func safeTLVReader(r io.Reader) (io.Reader, error) { + // Read at most maxConnMessageSize+1 so an over-cap message is + // detected without buffering an unbounded amount. + limited := io.LimitReader(r, maxConnMessageSize+1) + + buf, err := io.ReadAll(limited) + if err != nil { + return nil, fmt.Errorf("%w: read conn message: %v", + ErrInvalidTLV, err) + } + + if len(buf) > maxConnMessageSize { + return nil, fmt.Errorf("%w: message %d bytes exceeds max %d", + ErrInvalidTLV, len(buf), maxConnMessageSize) + } + + if err := validateTLVRecordLengths(buf); err != nil { + return nil, err + } + + return bytes.NewReader(buf), nil +} + +// validateTLVRecordLengths walks the (type, length, value) framing of +// a buffered TLV stream and rejects any record whose declared length +// exceeds the bytes remaining in the buffer. It deliberately does not +// interpret record contents; it only ensures the framing cannot drive +// an over-sized make() in the real decoder. +func validateTLVRecordLengths(buf []byte) error { + var scratch [8]byte + br := bytes.NewReader(buf) + + for br.Len() > 0 { + // The type varint may legitimately end the stream cleanly. + if _, err := tlv.ReadVarInt(br, &scratch); err != nil { + return fmt.Errorf("%w: read tlv type: %v", + ErrInvalidTLV, err) + } + + length, err := tlv.ReadVarInt(br, &scratch) + if err != nil { + return fmt.Errorf("%w: read tlv length: %v", + ErrInvalidTLV, err) + } + + // A record can never carry more value bytes than remain in + // the buffer; reject before the decoder allocates. + if length > uint64(br.Len()) { + return fmt.Errorf("%w: tlv record length %d exceeds "+ + "%d remaining bytes", ErrInvalidTLV, length, + br.Len()) + } + + if _, err := br.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return fmt.Errorf("%w: skip tlv value: %v", + ErrInvalidTLV, err) + } + } + + return nil +} diff --git a/mailbox/conn/safe_tlv_fuzz_test.go b/mailbox/conn/safe_tlv_fuzz_test.go new file mode 100644 index 000000000..04aafb74e --- /dev/null +++ b/mailbox/conn/safe_tlv_fuzz_test.go @@ -0,0 +1,126 @@ +package conn + +import ( + "bytes" + "testing" + + "github.com/lightningnetwork/lnd/tlv" + "google.golang.org/protobuf/types/known/wrapperspb" +) + +// FuzzAckStateDecode drives AckState.Decode with arbitrary bytes. The +// AckState blob is persisted in the durable checkpoint store and +// replayed from disk across restarts/upgrades, so a crafted record +// length must surface as an error rather than panicking the actor on +// every replay (regression for the tlv unbounded-make DoS). +func FuzzAckStateDecode(f *testing.F) { + // Seed with a valid encoding so the fuzzer explores mutations of + // real framing. + var buf bytes.Buffer + seed := &AckState{ + PullCursor: 42, + DispatchCommittedTo: 30, + AckTarget: 30, + AckCommittedTo: 20, + } + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + + // Known regression crashers: a tiny payload that declares a record + // length near 2^63 / 2^64 in a single type/length prefix. The + // first hits the unknown-odd-record path (stream.go make), the + // second the known-record path. + f.Add([]byte{ + 0x0b, + 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + f.Add([]byte{ + 0x03, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + f.Add([]byte{}) + + f.Fuzz(func(t *testing.T, data []byte) { + var v AckState + + // MUST NOT panic on any input. + if err := v.Decode(bytes.NewReader(data)); err != nil { + return + } + + // A successful decode must re-encode and re-decode cleanly so + // the persisted form round-trips. + var out bytes.Buffer + if err := v.Encode(&out); err != nil { + t.Fatalf("re-encode: %v", err) + } + + var v2 AckState + if err := v2.Decode(bytes.NewReader(out.Bytes())); err != nil { + t.Fatalf("re-decode: %v", err) + } + + if v != v2 { + t.Fatalf("round-trip mismatch: %+v != %+v", v, v2) + } + }) +} + +// decodeWrappedProtoStream is a test-only adapter that exercises the +// WrappedProto record decoder (which holds the make([]byte, l) proto +// allocation) through a bounded TLV reader, mirroring how serverconn +// messages decode their embedded proto payloads. +func decodeWrappedProtoStream(data []byte) error { + rec := tlv.ZeroRecordT[ + tlv.TlvType1, WrappedProto[*wrapperspb.StringValue], + ]() + rec.Val.Val = &wrapperspb.StringValue{} + + stream, err := tlv.NewStream(rec.Record()) + if err != nil { + return err + } + + safe, err := safeTLVReader(bytes.NewReader(data)) + if err != nil { + return err + } + + _, err = stream.DecodeWithParsedTypes(safe) + + return err +} + +// FuzzWrappedProtoDecode drives the WrappedProto record decoder, which +// sizes its value buffer with make([]byte, l) from an attacker +// controlled length. WrappedProto re-encodes proto bytes (not byte +// exact for arbitrary inputs), so we assert no-panic only rather than +// round-trip equality. +func FuzzWrappedProtoDecode(f *testing.F) { + rec := tlv.ZeroRecordT[ + tlv.TlvType1, WrappedProto[*wrapperspb.StringValue], + ]() + rec.Val.Val = wrapperspb.String("hello") + + stream, err := tlv.NewStream(rec.Record()) + if err == nil { + var buf bytes.Buffer + if err := stream.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + } + + // Known crasher: type 1 (the wrapped-proto field) with a huge + // declared length in a tiny payload. + f.Add([]byte{ + 0x01, + 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + f.Add([]byte{}) + + f.Fuzz(func(t *testing.T, data []byte) { + // MUST NOT panic; an error is the acceptable outcome. + _ = decodeWrappedProtoStream(data) + }) +} diff --git a/oor/actor_durable_message.go b/oor/actor_durable_message.go index 54a031009..ff5881158 100644 --- a/oor/actor_durable_message.go +++ b/oor/actor_durable_message.go @@ -235,8 +235,7 @@ func decodeStartTransferPayloadWithLimits(raw []byte, return startTransferPayload{}, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return startTransferPayload{}, err } @@ -501,8 +500,7 @@ func decodeAncestryEntry(raw []byte) (vtxo.Ancestry, error) { return vtxo.Ancestry{}, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return vtxo.Ancestry{}, err } @@ -704,8 +702,7 @@ func decodeIncomingMetadataMatchWithLimits(raw []byte, return IncomingMetadataMatch{}, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return IncomingMetadataMatch{}, err } @@ -837,8 +834,7 @@ func decodeRecipientPayload(raw []byte) (recipientPayload, error) { return recipientPayload{}, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return recipientPayload{}, err } @@ -1139,8 +1135,7 @@ func decodeTransferInputSnapshot(raw []byte) (*TransferInputSnapshot, error) { return nil, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return nil, err } @@ -1460,8 +1455,7 @@ func decodeSessionPayload(raw []byte) (SessionID, error) { return SessionID{}, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return SessionID{}, err } @@ -1524,8 +1518,7 @@ func decodeResumePayload(raw []byte) (SessionID, bool, error) { return SessionID{}, false, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return SessionID{}, false, err } @@ -1590,8 +1583,7 @@ func decodeListSessionsPayload(raw []byte) (SessionDirection, bool, error) { return SessionDirectionAll, false, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return SessionDirectionAll, false, err } @@ -1677,8 +1669,7 @@ func decodeResolveIncomingTransferPayloadWithLimits(raw []byte, return SessionID{}, nil, 0, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return SessionID{}, nil, 0, err } @@ -1747,8 +1738,7 @@ func decodeRestoreSnapshotPayloadWithLimits(raw []byte, return nil, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return nil, err } @@ -1821,8 +1811,7 @@ func decodeDriveEventRequestPayloadWithLimits(raw []byte, return SessionID{}, nil, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return SessionID{}, nil, err } @@ -2109,8 +2098,7 @@ func decodeEventPayloadWithLimits(raw []byte, return nil, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return nil, err } @@ -2367,6 +2355,14 @@ func decodeLengthPrefixedBlobListWithLimits(raw []byte, limits.MaxMailboxItems) } + // Each element occupies at least one length-prefix byte on the wire, so + // a count claiming more elements than the remaining bytes could back is + // corrupt. This bounds the make([][]byte, 0, count) capacity even + // though count is also capped by MaxMailboxItems. + if err := checkElemCount(reader.Len(), count, 1); err != nil { + return nil, fmt.Errorf("blob list count: %w", err) + } + blobs := make([][]byte, 0, count) for i := uint64(0); i < count; i++ { elementLen, err := tlv.ReadVarInt(reader, &scratch) @@ -2374,6 +2370,17 @@ func decodeLengthPrefixedBlobListWithLimits(raw []byte, return nil, err } + // Reject an element length that exceeds the bytes physically + // present before allocating make([]byte, elementLen). Without + // this a tiny payload declaring a huge length panics with + // "makeslice: cap out of range" or drives an OOM, reachable + // from attacker-controlled durable-mailbox bytes. + if elementLen > uint64(reader.Len()) { + return nil, fmt.Errorf("blob list element length %d "+ + "exceeds %d remaining bytes", elementLen, + reader.Len()) + } + element := make([]byte, elementLen) if _, err := io.ReadFull(reader, element); err != nil { return nil, err diff --git a/oor/decode_fuzz_test.go b/oor/decode_fuzz_test.go new file mode 100644 index 000000000..82923062f --- /dev/null +++ b/oor/decode_fuzz_test.go @@ -0,0 +1,331 @@ +package oor + +import ( + "bytes" + "testing" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/wire" + "github.com/lightningnetwork/lnd/tlv" +) + +// These fuzz tests target the OOR durable-message decoders, which run on +// attacker-controlled bytes that cross the client/server trust boundary and +// persist in durable actor mailboxes replayed across upgrades. The +// invariant under test is uniform: a decoder MUST return an error (never +// panic, OOM, or slice-OOB) on malformed input, and any value it accepts +// MUST round-trip back through its encoder. + +// seedBlobList encodes a small valid length-prefixed blob list so the fuzz +// corpus starts from a structurally valid seed. +func seedBlobList(tb testing.TB) []byte { + tb.Helper() + + raw, err := encodeLengthPrefixedBlobList([][]byte{ + {1, 2, 3}, {}, {4, 5}, + }) + if err != nil { + tb.Fatalf("encode blob list: %v", err) + } + + return raw +} + +// FuzzDecodeLengthPrefixedBlobList exercises the shared length-prefixed blob +// list decoder, the helper every nested OOR list decoder funnels through. +// Its per-element make([]byte, elementLen) is the primary unbounded-alloc +// sink reachable from a crafted count/length prefix. +func FuzzDecodeLengthPrefixedBlobList(f *testing.F) { + f.Add(seedBlobList(f)) + f.Add([]byte{}) + f.Add([]byte{0x00}) + + // A count of 1 followed by a huge declared element length in a tiny + // payload: the historical crasher for the unbounded make([]byte, ...). + f.Add([]byte{0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}) + + f.Fuzz(func(t *testing.T, data []byte) { + blobs, err := decodeLengthPrefixedBlobList(data) + if err != nil { + return + } + + // A value that decoded must re-encode and re-decode cleanly. + out, err := encodeLengthPrefixedBlobList(blobs) + if err != nil { + t.Fatalf("re-encode: %v", err) + } + + if _, err := decodeLengthPrefixedBlobList(out); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzDecodeOutgoingSnapshot exercises the outgoing-snapshot TLV decoder, +// the top-level durable state blob for an outgoing OOR transfer. +func FuzzDecodeOutgoingSnapshot(f *testing.F) { + snap := &OutgoingSnapshot{ + Version: 4, + SessionID: SessionID(chainhash.Hash{1, 2, 3}), + Phase: OutgoingPhaseSubmitSent, + ArkPSBT: []byte{1, 2, 3, 4}, + CheckpointPSBTs: [][]byte{{5, 6}, {7, 8}}, + FailReason: "boom", + IdempotencyKey: "key", + } + if raw, err := encodeOutgoingSnapshot(snap); err == nil { + f.Add(raw) + } + f.Add([]byte{}) + + f.Fuzz(func(t *testing.T, data []byte) { + got, err := decodeOutgoingSnapshot(data) + if err != nil { + return + } + + out, err := encodeOutgoingSnapshot(got) + if err != nil { + t.Fatalf("re-encode: %v", err) + } + + if _, err := decodeOutgoingSnapshot(out); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzDecodeIncomingSnapshot exercises the incoming-snapshot TLV decoder, +// the top-level durable state blob for an incoming OOR transfer (carries +// ancestor packages and checkpoint lists). +func FuzzDecodeIncomingSnapshot(f *testing.F) { + snap := &IncomingSnapshot{ + Version: 1, + SessionID: SessionID(chainhash.Hash{9, 8, 7}), + Phase: IncomingPhaseResolvePending, + ArkPSBT: []byte{1, 2}, + CheckpointPSBTs: [][]byte{{3, 4}}, + FailReason: "nope", + RecipientPkScript: []byte{0xaa, 0xbb}, + RecipientEventID: 42, + MetadataAttempts: 2, + ResolveAttempts: 1, + } + if raw, err := encodeIncomingSnapshot(snap); err == nil { + f.Add(raw) + } + f.Add([]byte{}) + + f.Fuzz(func(t *testing.T, data []byte) { + got, err := decodeIncomingSnapshotWithLimits( + data, DefaultReceiveLimits(), + ) + if err != nil { + return + } + + // AncestorPackages contains *psbt.Packet values whose re-encode + // is not guaranteed byte-exact, so assert no-panic re-encode + // rather than byte equality. + if _, err := encodeIncomingSnapshot(got); err != nil { + t.Fatalf("re-encode: %v", err) + } + }) +} + +// FuzzDecodeStartTransferPayload exercises the start-transfer payload +// decoder, which carries the operator key, transfer inputs, and recipients +// of an outgoing OOR send request. +func FuzzDecodeStartTransferPayload(f *testing.F) { + f.Add([]byte{}) + f.Add([]byte{0x01, 0x00}) + f.Fuzz(func(t *testing.T, data []byte) { + // Decode under bounded limits; the contract is no-panic. + _, _ = decodeStartTransferPayloadWithLimits( + data, DefaultReceiveLimits(), + ) + }) +} + +// FuzzDecodePackageArtifact exercises the single package-artifact TLV +// decoder, which carries a session id, an Ark PSBT, and a checkpoint list. +func FuzzDecodePackageArtifact(f *testing.F) { + f.Add([]byte{}) + f.Fuzz(func(t *testing.T, data []byte) { + _, _ = decodePackageArtifactWithLimits( + data, DefaultReceiveLimits(), + ) + }) +} + +// FuzzDecodeConditionWitness exercises the hand-rolled condition-witness +// list decoder (wire.ReadVarInt count + wire.ReadVarBytes items). +func FuzzDecodeConditionWitness(f *testing.F) { + if raw, err := encodeConditionWitness( + [][]byte{{1, 2}, {3}}, + ); err == nil { + f.Add(raw) + } + f.Add([]byte{}) + f.Add([]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}) + + f.Fuzz(func(t *testing.T, data []byte) { + _, _ = decodeConditionWitness(data) + }) +} + +// FuzzDecodeExternalSignatures exercises the external-signature list decoder +// (count varint plus three var-byte fields and a fixed sighash per item). +func FuzzDecodeExternalSignatures(f *testing.F) { + f.Add([]byte{}) + f.Add([]byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}) + f.Fuzz(func(t *testing.T, data []byte) { + _, _ = decodeExternalSignatures(data) + }) +} + +// FuzzDecodeOutPointList exercises the outpoint-list decoder, which sizes a +// wire.OutPoint slice from a count prefix. +func FuzzDecodeOutPointList(f *testing.F) { + if raw, err := encodeOutPointList([]wire.OutPoint{ + {Hash: chainhash.Hash{1}, Index: 7}, + }); err == nil { + f.Add(raw) + } + f.Add([]byte{}) + f.Fuzz(func(t *testing.T, data []byte) { + _, _ = decodeOutPointListWithLimits(data, DefaultReceiveLimits()) + }) +} + +// FuzzDecodeRestoreSnapshotPayload exercises the restore-payload decoder +// which wraps an embedded outgoing snapshot in an outer TLV record. +func FuzzDecodeRestoreSnapshotPayload(f *testing.F) { + f.Add([]byte{}) + f.Fuzz(func(t *testing.T, data []byte) { + _, _ = decodeRestoreSnapshotPayloadWithLimits( + data, DefaultReceiveLimits(), + ) + }) +} + +// FuzzDecodeEventPayload exercises the FSM event-payload decoder, the body +// of every durable DriveEventRequest. Its event-kind switch fans out into +// PSBT lists, ancestor packages, metadata matches, and outpoint lists, so it +// is the single richest attacker-controlled decode surface in the package and +// is not otherwise reached by the top-level snapshot decoders. The contract +// is no-panic: most event kinds carry *psbt.Packet values whose re-encode is +// not byte-exact, so a round-trip assertion is not meaningful here. +func FuzzDecodeEventPayload(f *testing.F) { + // A FailEvent has no PSBT payload, so it both encodes and decodes + // cleanly and gives the fuzzer a structurally valid seed for the + // event-kind switch. + if raw, err := encodeEventPayload( + &FailEvent{Reason: "boom"}, + ); err == nil { + f.Add(raw) + } + f.Add([]byte{}) + f.Add([]byte{0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}) + + f.Fuzz(func(t *testing.T, data []byte) { + _, _ = decodeEventPayloadWithLimits(data, DefaultReceiveLimits()) + }) +} + +// FuzzDecodeDriveEventRequestPayload exercises the outer DriveEventRequest +// payload decoder (session id + embedded event payload). DriveEventRequest is +// a TLV-durable message replayed from the mailbox, so its framing crosses the +// trust boundary independently of the inner event body fuzzed above. +func FuzzDecodeDriveEventRequestPayload(f *testing.F) { + f.Add([]byte{}) + f.Add([]byte{0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}) + f.Fuzz(func(t *testing.T, data []byte) { + _, _, _ = decodeDriveEventRequestPayloadWithLimits( + data, DefaultReceiveLimits(), + ) + }) +} + +// FuzzDecodeRecipientPayload exercises the single-recipient TLV decoder, which +// narrows an attacker-supplied uint64 value into int64 and carries two +// variable-length byte fields. It is reached only nested under start-transfer +// today; a direct target lets the fuzzer explore its value-overflow guard and +// byte-field framing without first constructing a valid outer list. +func FuzzDecodeRecipientPayload(f *testing.F) { + if raw, err := encodeRecipientPayload(recipientPayload{ + PkScript: []byte{0xaa, 0xbb}, + ValueSat: 1234, + VTXOPolicyTemplate: []byte{0x01}, + }); err == nil { + f.Add(raw) + } + f.Add([]byte{}) + f.Add([]byte{0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}) + + f.Fuzz(func(t *testing.T, data []byte) { + got, err := decodeRecipientPayload(data) + if err != nil { + return + } + + out, err := encodeRecipientPayload(got) + if err != nil { + t.Fatalf("re-encode: %v", err) + } + + if _, err := decodeRecipientPayload(out); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzDecodeUint32List exercises the hand-rolled big-endian uint32 list +// decoder used inside ancestry entries. It sizes make([]uint32, count) from a +// 4-byte count prefix and is the package's only length-prefixed list decoder +// that bypasses checkElemCount, relying instead on an explicit +// implied-length equality check; a direct target stresses that bound against +// crafted count/length mismatches that could otherwise wrap on 32-bit +// platforms. +func FuzzDecodeUint32List(f *testing.F) { + f.Add(encodeUint32List([]uint32{1, 2, 3})) + f.Add([]byte{}) + + // A count of 0xffffffff with no backing bytes: the over-allocation / + // int-wrap probe for the make([]uint32, count) sink. + f.Add([]byte{0xff, 0xff, 0xff, 0xff}) + + f.Fuzz(func(t *testing.T, data []byte) { + got, err := decodeUint32List(data) + if err != nil { + return + } + + // A decoded list must re-encode and re-decode byte-stably. + if _, err := decodeUint32List(encodeUint32List(got)); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzValidateTLVRecordLengths exercises the framing pre-validator directly +// so a malformed varint or over-declared record length is rejected without +// panicking before the real decoder ever allocates. +func FuzzValidateTLVRecordLengths(f *testing.F) { + var buf bytes.Buffer + val := []byte{1, 2, 3} + rec := tlv.MakePrimitiveRecord(1, &val) + if stream, err := tlv.NewStream(rec); err == nil { + if err := stream.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + } + f.Add([]byte{}) + f.Add([]byte{0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}) + + f.Fuzz(func(t *testing.T, data []byte) { + // Pure no-panic contract; either outcome is acceptable. + _ = validateTLVRecordLengths(data) + }) +} diff --git a/oor/outgoing_snapshot_codec.go b/oor/outgoing_snapshot_codec.go index 2554f5dff..57cdda853 100644 --- a/oor/outgoing_snapshot_codec.go +++ b/oor/outgoing_snapshot_codec.go @@ -139,8 +139,7 @@ func decodeOutgoingSnapshotWithLimits(raw []byte, return nil, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return nil, err } diff --git a/oor/package_artifact_codec.go b/oor/package_artifact_codec.go index c4541da73..bab013af6 100644 --- a/oor/package_artifact_codec.go +++ b/oor/package_artifact_codec.go @@ -148,8 +148,7 @@ func decodePackageArtifactWithLimits(raw []byte, return PackageArtifact{}, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return PackageArtifact{}, err } diff --git a/oor/receive_snapshot.go b/oor/receive_snapshot.go index f45aa7b3c..5a6e43241 100644 --- a/oor/receive_snapshot.go +++ b/oor/receive_snapshot.go @@ -394,8 +394,7 @@ func decodeIncomingSnapshotWithLimits(raw []byte, return nil, err } - reader := bytes.NewReader(raw) - if _, err := stream.DecodeWithParsedTypes(reader); err != nil { + if _, err := decodeBoundedStream(stream, raw); err != nil { return nil, err } diff --git a/oor/tlv_bounds.go b/oor/tlv_bounds.go new file mode 100644 index 000000000..5b4c14927 --- /dev/null +++ b/oor/tlv_bounds.go @@ -0,0 +1,156 @@ +package oor + +import ( + "bytes" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// maxOORMessageSize bounds the total wire size of an untrusted OOR TLV +// message before any individual record length is honored. OOR messages +// carry PSBT-bearing payloads (Ark + checkpoint transactions, ancestor +// packages), so the cap is generous at 16 MiB while still rejecting the +// unbounded-allocation DoS where a tiny payload declares a multi-gigabyte +// (or near-2^64) record length. These payloads cross the client/server +// trust boundary and persist in durable actor mailboxes replayed across +// upgrades, so the bound is load-bearing on both the live and replay path. +const maxOORMessageSize = 16 << 20 + +// safeOORTLVReader reads an untrusted OOR TLV payload into a size-capped +// buffer and pre-validates the (type, length) framing so the downstream +// tlv.Stream.DecodeWithParsedTypes can never be handed a record length +// larger than the bytes physically present in the message. +// +// This is required because the tlv library sizes its value buffers with +// make([]byte, declaredLength) BEFORE reading any value bytes: stream.go's +// unknown-record path does bytes.NewBuffer(make([]byte, 0, length)) and +// primitive.go's DVarBytes does make([]byte, l). Neither bounds length +// against the input on the non-P2P Decode path. A producer-declared length +// near 2^64 panics the decoder with "makeslice: cap out of range", and a +// multi-gigabyte length drives an OOM. By rejecting any record whose +// declared length exceeds the remaining buffer (a record can never +// legitimately carry more bytes than the message contains), we cap every +// allocation at the message size, which is itself capped at +// maxOORMessageSize. +func safeOORTLVReader(r io.Reader) (*bytes.Reader, error) { + // Read at most maxOORMessageSize+1 so an over-cap message is detected + // without buffering an unbounded amount. + limited := io.LimitReader(r, maxOORMessageSize+1) + buf, err := io.ReadAll(limited) + if err != nil { + return nil, fmt.Errorf("read oor message: %w", err) + } + + if len(buf) > maxOORMessageSize { + return nil, fmt.Errorf("oor message %d bytes exceeds max %d", + len(buf), maxOORMessageSize) + } + + if err := validateTLVRecordLengths(buf); err != nil { + return nil, err + } + + return bytes.NewReader(buf), nil +} + +// safeOORTLVBytes validates an in-memory OOR TLV blob and returns a reader +// positioned at its start. It is the []byte-input analog of +// safeOORTLVReader for the many nested decoders that already receive a +// fully-buffered payload extracted from a parent record. +func safeOORTLVBytes(raw []byte) (*bytes.Reader, error) { + if len(raw) > maxOORMessageSize { + return nil, fmt.Errorf("oor message %d bytes exceeds max %d", + len(raw), maxOORMessageSize) + } + + if err := validateTLVRecordLengths(raw); err != nil { + return nil, err + } + + return bytes.NewReader(raw), nil +} + +// decodeBoundedStream pre-validates the (type, length) framing of an +// in-memory OOR TLV blob and only then decodes it through the supplied +// stream. Pre-validation guarantees the tlv decoder is never handed a +// record length larger than the bytes present, so its internal +// make([]byte, declaredLength) allocations are bounded by the blob size +// (itself capped at maxOORMessageSize). It returns the parsed type map so +// callers that inspect record presence keep their existing behavior. +// +// This is the shared choke point every oor TLV decode routes through so +// the bound holds uniformly across the many heterogeneous decoders without +// duplicating the validation at each call site. +func decodeBoundedStream(stream *tlv.Stream, raw []byte) (tlv.TypeMap, + error) { + + reader, err := safeOORTLVBytes(raw) + if err != nil { + return nil, err + } + + return stream.DecodeWithParsedTypes(reader) +} + +// validateTLVRecordLengths walks the (type, length, value) framing of a +// buffered TLV stream and rejects any record whose declared length exceeds +// the bytes remaining in the buffer. It deliberately does not interpret +// record contents; it only ensures the framing cannot drive an over-sized +// make() in the real decoder before any value bytes are read. +func validateTLVRecordLengths(buf []byte) error { + var scratch [8]byte + br := bytes.NewReader(buf) + + for br.Len() > 0 { + // A clean end of stream is signaled by the type varint read + // reaching EOF, which only happens when br.Len() == 0 above; + // any read failure here is a malformed stream. + if _, err := tlv.ReadVarInt(br, &scratch); err != nil { + return fmt.Errorf("read tlv type: %w", err) + } + + length, err := tlv.ReadVarInt(br, &scratch) + if err != nil { + return fmt.Errorf("read tlv length: %w", err) + } + + // A record can never carry more value bytes than remain in the + // buffer; reject before the decoder allocates make([]byte, + // length). + if length > uint64(br.Len()) { + return fmt.Errorf("tlv record length %d exceeds %d "+ + "remaining bytes", length, br.Len()) + } + + if _, err := br.Seek(int64(length), io.SeekCurrent); err != nil { + return fmt.Errorf("skip tlv value: %w", err) + } + } + + return nil +} + +// checkElemCount rejects a decoded element count that cannot possibly be +// backed by the bytes physically present in buf, before any make([]T, +// count) allocation. minBytesPerElem is the smallest on-wire size a single +// element can occupy; a count claiming more elements than buf could hold is +// a corrupt or malicious framing and is rejected. This guards the +// hand-rolled length-prefixed list decoders that size their output slice +// from an attacker-controlled count varint. +func checkElemCount(bufLen int, count uint64, minBytesPerElem uint64) error { + if minBytesPerElem == 0 { + minBytesPerElem = 1 + } + + // max is the largest element count buf could physically back. Computed + // in uint64 so the division cannot overflow. + max := uint64(bufLen) / minBytesPerElem + if count > max { + return fmt.Errorf("element count %d exceeds %d backable by "+ + "%d bytes", count, max, bufLen) + } + + return nil +} diff --git a/serverconn/actor.go b/serverconn/actor.go index 9af72673d..7fe2264fb 100644 --- a/serverconn/actor.go +++ b/serverconn/actor.go @@ -303,7 +303,16 @@ func (m *SendClientEventRequest) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode: this wrapper persists + // in the durable outbox and is replayed from disk, so a crafted + // record length must not drive an unbounded make() in the tlv + // library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return err } @@ -462,7 +471,14 @@ func (m *SendRPCRequest) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return err } @@ -622,7 +638,14 @@ func (m *SendUnaryRequest) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return err } diff --git a/serverconn/durable_unary_queries.go b/serverconn/durable_unary_queries.go index e0671c794..f78e08313 100644 --- a/serverconn/durable_unary_queries.go +++ b/serverconn/durable_unary_queries.go @@ -219,7 +219,16 @@ func (m *SendListOORRecipientEventsByScriptRequest) Decode(r io.Reader) error { return err } - if _, err := stream.DecodeWithParsedTypes(r); err != nil { + // Bound the untrusted payload before decode: this durable query + // persists in the outbox and is replayed from disk, so a crafted + // record length must not drive an unbounded make() in the tlv + // library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + if _, err := stream.DecodeWithParsedTypes(safe); err != nil { return err } @@ -425,7 +434,14 @@ func (m *SendListVTXOsByScriptsRequest) Decode(r io.Reader) error { return err } - parsed, err := stream.DecodeWithParsedTypes(r) + // Bound the untrusted payload before decode so a crafted record + // length cannot drive an unbounded make() in the tlv library. + safe, err := safeTLVReader(r) + if err != nil { + return err + } + + parsed, err := stream.DecodeWithParsedTypes(safe) if err != nil { return err } @@ -585,6 +601,16 @@ func decodeLengthPrefixedBlobList(raw []byte) ([][]byte, error) { return nil, err } + // Each blob carries at least its own 4-byte length prefix, so a + // count larger than the remaining bytes can never be satisfied. + // Bound it before make([][]byte, 0, count) so a crafted count (up + // to 4 GiB for a uint32) cannot pre-allocate a multi-gigabyte + // backing array and OOM the actor on replay. + if uint64(count) > uint64(reader.Len())/4 { + return nil, fmt.Errorf("blob count %d exceeds %d remaining "+ + "bytes", count, reader.Len()) + } + blobs := make([][]byte, 0, count) for i := uint32(0); i < count; i++ { blob, err := readLengthPrefixedBlob(reader) @@ -614,13 +640,23 @@ func writeLengthPrefixedBlob(w io.Writer, blob []byte) error { return err } -// readLengthPrefixedBlob decodes one length-prefixed byte slice. +// readLengthPrefixedBlob decodes one length-prefixed byte slice. The +// declared size is bounded against the bytes physically remaining in +// the reader before make([]byte, size) so a crafted length (up to 4 +// GiB for a uint32 prefix) cannot drive an OOM ahead of io.ReadFull. +// A blob can never legitimately carry more bytes than remain in its +// enclosing buffer. func readLengthPrefixedBlob(r *bytes.Reader) ([]byte, error) { var size uint32 if err := binary.Read(r, binary.BigEndian, &size); err != nil { return nil, err } + if uint64(size) > uint64(r.Len()) { + return nil, fmt.Errorf("blob length %d exceeds %d remaining "+ + "bytes", size, r.Len()) + } + blob := make([]byte, size) if _, err := io.ReadFull(r, blob); err != nil { return nil, err diff --git a/serverconn/safe_tlv.go b/serverconn/safe_tlv.go new file mode 100644 index 000000000..e5e4bae4a --- /dev/null +++ b/serverconn/safe_tlv.go @@ -0,0 +1,107 @@ +package serverconn + +import ( + "bytes" + "errors" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// ErrInvalidTLV is the sentinel wrapped by the bounded TLV decode +// helpers so callers can classify a malformed or oversized payload as +// an external decode failure rather than an internal bug. +var ErrInvalidTLV = errors.New("invalid tlv payload") + +// maxServerConnMessageSize bounds the total wire size of an untrusted +// serverconn TLV payload before any individual record length is +// honored. These messages wrap mailbox envelopes / Any-packed proto +// bodies plus a handful of length-prefixed identifier blobs, all of +// which are bounded by the mailbox transport. 4 MiB gives generous +// headroom for the largest legitimate proto body while still +// rejecting the unbounded-allocation DoS where a tiny payload declares +// a multi-gigabyte (or near-2^64) record length and crosses the +// client/server trust boundary into the durable mailbox. +const maxServerConnMessageSize = 4 << 20 + +// safeTLVReader reads an untrusted TLV payload into a size-capped +// buffer and pre-validates the (type, length, value) framing so the +// downstream tlv.Stream.DecodeWithParsedTypes can never be handed a +// record length larger than the bytes physically present. +// +// This is load-bearing because the tlv library sizes its value buffers +// with make([]byte, declaredLength) BEFORE reading any value bytes: +// stream.go does bytes.NewBuffer(make([]byte, 0, length)) for unknown +// records, and primitive.go's DVarBytes does make([]byte, l) for known +// []byte fields. A producer-declared length near 2^64 panics the +// decoder with "makeslice: cap out of range", and a multi-gigabyte +// length drives an OOM. Both are reachable from a few attacker +// controlled bytes that persist in the durable mailbox and are +// replayed from disk across upgrades. By rejecting any record whose +// declared length exceeds the remaining buffer we cap every allocation +// at the message size, which is itself capped at +// maxServerConnMessageSize. +func safeTLVReader(r io.Reader) (io.Reader, error) { + // Read at most maxServerConnMessageSize+1 so an over-cap message is + // detected without buffering an unbounded amount. + limited := io.LimitReader(r, maxServerConnMessageSize+1) + + buf, err := io.ReadAll(limited) + if err != nil { + return nil, fmt.Errorf("%w: read serverconn message: %v", + ErrInvalidTLV, err) + } + + if len(buf) > maxServerConnMessageSize { + return nil, fmt.Errorf("%w: message %d bytes exceeds max %d", + ErrInvalidTLV, len(buf), maxServerConnMessageSize) + } + + if err := validateTLVRecordLengths(buf); err != nil { + return nil, err + } + + return bytes.NewReader(buf), nil +} + +// validateTLVRecordLengths walks the (type, length, value) framing of +// a buffered TLV stream and rejects any record whose declared length +// exceeds the bytes remaining in the buffer. It deliberately does not +// interpret record contents; it only ensures the framing cannot drive +// an over-sized make() in the real decoder. +func validateTLVRecordLengths(buf []byte) error { + var scratch [8]byte + br := bytes.NewReader(buf) + + for br.Len() > 0 { + // The type varint may legitimately end the stream cleanly. + if _, err := tlv.ReadVarInt(br, &scratch); err != nil { + return fmt.Errorf("%w: read tlv type: %v", + ErrInvalidTLV, err) + } + + length, err := tlv.ReadVarInt(br, &scratch) + if err != nil { + return fmt.Errorf("%w: read tlv length: %v", + ErrInvalidTLV, err) + } + + // A record can never carry more value bytes than remain in + // the buffer; reject before the decoder allocates. + if length > uint64(br.Len()) { + return fmt.Errorf("%w: tlv record length %d exceeds "+ + "%d remaining bytes", ErrInvalidTLV, length, + br.Len()) + } + + if _, err := br.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return fmt.Errorf("%w: skip tlv value: %v", + ErrInvalidTLV, err) + } + } + + return nil +} diff --git a/serverconn/safe_tlv_fuzz_test.go b/serverconn/safe_tlv_fuzz_test.go new file mode 100644 index 000000000..592a731db --- /dev/null +++ b/serverconn/safe_tlv_fuzz_test.go @@ -0,0 +1,170 @@ +package serverconn + +import ( + "bytes" + "testing" +) + +// hugeRecordPayloads returns a set of tiny TLV payloads that each +// declare a record length near 2^63 / 2^64 in their length prefix. +// These are the canonical crashers for the tlv unbounded-make DoS: +// the first byte is a small type, followed by an 0xff-prefixed varint +// that decodes to a giant length. They MUST be rejected (error) rather +// than panicking the decoder. +func hugeRecordPayloads() [][]byte { + return [][]byte{ + // Unknown odd type 11, length ~2^63 (stream.go make path). + {0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + // Type 1 (a known []byte field on every message), giant len. + {0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + // Type 0 (the proto payload on event/unary), giant len. + {0x00, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + {}, + } +} + +// FuzzSendClientEventRequestDecode drives SendClientEventRequest.Decode +// with arbitrary bytes. The wrapper persists in the durable outbox and +// is replayed from disk, so a crafted payload must error, not panic. +// Its Encode re-marshals an embedded proto, so we assert no-panic only. +func FuzzSendClientEventRequestDecode(f *testing.F) { + seed := &SendClientEventRequest{ + Message: &bytesServerMessage{payload: []byte("evt")}, + } + + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + for _, p := range hugeRecordPayloads() { + f.Add(p) + } + + f.Fuzz(func(t *testing.T, data []byte) { + var v SendClientEventRequest + + // MUST NOT panic; an error is the acceptable outcome. + _ = v.Decode(bytes.NewReader(data)) + }) +} + +// FuzzSendRPCRequestDecode drives SendRPCRequest.Decode (mailbox +// envelope wrapper). Encode re-marshals the envelope proto, so this +// asserts no-panic only. +func FuzzSendRPCRequestDecode(f *testing.F) { + for _, p := range hugeRecordPayloads() { + f.Add(p) + } + + f.Fuzz(func(t *testing.T, data []byte) { + var v SendRPCRequest + + // MUST NOT panic. + _ = v.Decode(bytes.NewReader(data)) + }) +} + +// FuzzSendUnaryRequestDecode drives SendUnaryRequest.Decode (Any-packed +// body plus routing metadata). Encode re-marshals proto, so this +// asserts no-panic only. +func FuzzSendUnaryRequestDecode(f *testing.F) { + for _, p := range hugeRecordPayloads() { + f.Add(p) + } + + f.Fuzz(func(t *testing.T, data []byte) { + var v SendUnaryRequest + + // MUST NOT panic. + _ = v.Decode(bytes.NewReader(data)) + }) +} + +// FuzzSendListOORRecipientEventsByScriptRequestDecode drives the +// recipient-events durable query decoder. Its fields are pure scalars +// and byte blobs, so a successful decode must round-trip byte-stably. +func FuzzSendListOORRecipientEventsByScriptRequestDecode(f *testing.F) { + seed := &SendListOORRecipientEventsByScriptRequest{ + PkScript: bytes.Repeat([]byte{0x02}, 34), + AfterEventID: 7, + Limit: 100, + CorrelationID: "corr-1", + MsgID: "msg-1", + } + + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + for _, p := range hugeRecordPayloads() { + f.Add(p) + } + + f.Fuzz(func(t *testing.T, data []byte) { + var v SendListOORRecipientEventsByScriptRequest + + // MUST NOT panic on any input. + if err := v.Decode(bytes.NewReader(data)); err != nil { + return + } + + var out bytes.Buffer + if err := v.Encode(&out); err != nil { + t.Fatalf("re-encode: %v", err) + } + + var v2 SendListOORRecipientEventsByScriptRequest + if err := v2.Decode(bytes.NewReader(out.Bytes())); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzSendListVTXOsByScriptsRequestDecode drives the VTXOs-by-scripts +// durable query decoder, which carries a count-prefixed, +// length-prefixed blob list (readLengthPrefixedBlob's make([]byte, +// size)) and an opaque cursor. Successful decodes round-trip via +// re-encode/re-decode (Encode validates the cursor, so a decoded value +// that fails re-encode is acceptable and skipped). +func FuzzSendListVTXOsByScriptsRequestDecode(f *testing.F) { + seed := &SendListVTXOsByScriptsRequest{ + PkScripts: [][]byte{ + bytes.Repeat([]byte{0x03}, 34), + bytes.Repeat([]byte{0x04}, 34), + }, + Limit: 50, + CorrelationID: "corr-2", + MsgID: "msg-2", + } + + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + for _, p := range hugeRecordPayloads() { + f.Add(p) + } + + f.Fuzz(func(t *testing.T, data []byte) { + var v SendListVTXOsByScriptsRequest + + // MUST NOT panic on any input. + if err := v.Decode(bytes.NewReader(data)); err != nil { + return + } + + // Re-encode may legitimately fail (Encode re-validates the + // opaque cursor), so a re-encode error is acceptable; only a + // panic or a re-decode failure of successfully re-encoded + // bytes is a bug. + var out bytes.Buffer + if err := v.Encode(&out); err != nil { + return + } + + var v2 SendListVTXOsByScriptsRequest + if err := v2.Decode(bytes.NewReader(out.Bytes())); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} diff --git a/unroll/codec_fuzz_test.go b/unroll/codec_fuzz_test.go new file mode 100644 index 000000000..f4c13ff7c --- /dev/null +++ b/unroll/codec_fuzz_test.go @@ -0,0 +1,268 @@ +package unroll + +import ( + "bytes" + "testing" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/darepo-client/unrollplan" +) + +// canonicalMaliciousTLV is the standard over-allocation probe: TLV type 11, +// then a BigSize-encoded length of math.MaxInt64. Without the safeTLVReader +// framing guard the decoder would size make([]byte, declaredLength) from this +// value and panic (makeslice) or OOM. Seeding it into every TLV-framed target +// pins the guard's rejection path into the persistent corpus. +var canonicalMaliciousTLV = []byte{ + 0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, +} + +// fuzzDecodeMsg is a small generic helper that exercises one durable mailbox +// message decoder against attacker-controlled bytes and, on a clean decode, +// re-encodes/re-decodes to confirm the value is self-consistent. The durable +// mailbox replays these payloads from disk across rolling upgrades, so a +// decode must never panic. +func fuzzDecodeMsg[T Msg](t *testing.T, data []byte, mk func() T) { + v := mk() + if err := v.Decode(bytes.NewReader(data)); err != nil { + return + } + + var out bytes.Buffer + if err := v.Encode(&out); err != nil { + t.Fatalf("re-encode: %v", err) + } + + v2 := mk() + if err := v2.Decode(bytes.NewReader(out.Bytes())); err != nil { + t.Fatalf("re-decode: %v", err) + } +} + +// FuzzStartUnrollRequestDecode fuzzes the StartUnrollRequest mailbox decoder. +// Its ExitPolicyKind/ExitPolicyRef records decode through tlv DVarBytes, the +// unbounded-allocation path. +func FuzzStartUnrollRequestDecode(f *testing.F) { + seed := &StartUnrollRequest{ + Height: 42, + Trigger: TriggerFraudSpend, + ExitPolicyKind: ExitPolicyKind("custom"), + ExitPolicyRef: "ref-123", + } + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + f.Add(canonicalMaliciousTLV) + + f.Fuzz(func(t *testing.T, data []byte) { + fuzzDecodeMsg(t, data, func() *StartUnrollRequest { + return &StartUnrollRequest{} + }) + }) +} + +// FuzzTxFailedMsgDecode fuzzes TxFailedMsg, whose Reason record decodes through +// the unbounded DVarBytes path. +func FuzzTxFailedMsgDecode(f *testing.F) { + seed := &TxFailedMsg{ + Txid: chainhash.Hash{0x01, 0x02}, + Reason: "broadcast rejected", + } + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + f.Add(canonicalMaliciousTLV) + + f.Fuzz(func(t *testing.T, data []byte) { + fuzzDecodeMsg(t, data, func() *TxFailedMsg { + return &TxFailedMsg{} + }) + }) +} + +// FuzzSpendObservedMsgDecode fuzzes SpendObservedMsg, which carries multiple +// fixed-size records that must tolerate truncation and length lies. +func FuzzSpendObservedMsgDecode(f *testing.F) { + seed := &SpendObservedMsg{ + Outpoint: wire.OutPoint{ + Hash: chainhash.Hash{0xaa}, + Index: 7, + }, + SpendingTxid: chainhash.Hash{0xbb}, + SpendingHeight: 100, + } + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + f.Add(canonicalMaliciousTLV) + + f.Fuzz(func(t *testing.T, data []byte) { + fuzzDecodeMsg(t, data, func() *SpendObservedMsg { + return &SpendObservedMsg{} + }) + }) +} + +// FuzzResumeUnrollRequestDecode fuzzes the ResumeUnrollRequest mailbox decoder. +// It carries a single fixed-size height record but still routes through +// safeDecodeStream, so a length lie in the framing must fail closed. +func FuzzResumeUnrollRequestDecode(f *testing.F) { + seed := &ResumeUnrollRequest{Height: 99} + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + f.Add(canonicalMaliciousTLV) + + f.Fuzz(func(t *testing.T, data []byte) { + fuzzDecodeMsg(t, data, func() *ResumeUnrollRequest { + return &ResumeUnrollRequest{} + }) + }) +} + +// FuzzHeightObservedMsgDecode fuzzes the HeightObservedMsg mailbox decoder. +func FuzzHeightObservedMsgDecode(f *testing.F) { + seed := &HeightObservedMsg{Height: 12345} + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + f.Add(canonicalMaliciousTLV) + + f.Fuzz(func(t *testing.T, data []byte) { + fuzzDecodeMsg(t, data, func() *HeightObservedMsg { + return &HeightObservedMsg{} + }) + }) +} + +// FuzzTxConfirmedMsgDecode fuzzes the TxConfirmedMsg mailbox decoder. Its +// fixed-size txid record must tolerate truncation and declared-length lies. +func FuzzTxConfirmedMsgDecode(f *testing.F) { + seed := &TxConfirmedMsg{ + Txid: chainhash.Hash{0xab, 0xcd}, + Height: 77, + NumConfs: 6, + } + var buf bytes.Buffer + if err := seed.Encode(&buf); err == nil { + f.Add(buf.Bytes()) + } + f.Add([]byte{}) + f.Add(canonicalMaliciousTLV) + + f.Fuzz(func(t *testing.T, data []byte) { + fuzzDecodeMsg(t, data, func() *TxConfirmedMsg { + return &TxConfirmedMsg{} + }) + }) +} + +// FuzzDecodeCheckpoint fuzzes the full actor checkpoint codec. The checkpoint +// embeds the planner state, an optional serialized sweep tx, the deferred +// checkpoint list, and several DVarBytes records — every unbounded-allocation +// vector in the package converges here. +func FuzzDecodeCheckpoint(f *testing.F) { + state := unrollplan.State{ + ConfirmedTxids: []chainhash.Hash{{0x01}}, + InFlightTxids: []chainhash.Hash{{0x02}}, + } + cp := &actorCheckpoint{ + Version: checkpointVersion, + Height: 123, + Started: true, + Trigger: TriggerManual, + State: state, + ExitPolicyKind: ExitPolicyKind("custom"), + ExitPolicyRef: "ref", + Fail: "boom", + SweepAttempts: 2, + DeferredCheckpoints: []DeferredCheckpoint{ + {Txid: chainhash.Hash{0x03}, DeadlineHeight: 9}, + }, + } + if seed, err := encodeCheckpoint(cp); err == nil { + f.Add(seed) + } + f.Add([]byte{}) + f.Add(canonicalMaliciousTLV) + + f.Fuzz(func(t *testing.T, data []byte) { + decoded, err := decodeCheckpoint(data) + if err != nil { + return + } + + out, err := encodeCheckpoint(decoded) + if err != nil { + t.Fatalf("re-encode: %v", err) + } + + if _, err := decodeCheckpoint(out); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzDecodeDeferredCheckpoints targets the deferred-checkpoint list codec +// directly. Its leading varint count drives make([]DeferredCheckpoint, 0, +// count) and each per-entry varint length drives make([]byte, entryLen) — both +// sized from attacker input. +func FuzzDecodeDeferredCheckpoints(f *testing.F) { + if seed, err := encodeDeferredCheckpoints([]DeferredCheckpoint{ + {Txid: chainhash.Hash{0x01}, DeadlineHeight: 1}, + {Txid: chainhash.Hash{0x02}, DeadlineHeight: 2}, + }); err == nil { + f.Add(seed) + } + f.Add([]byte{}) + + // Canonical malicious payload: a BigSize-encoded count of math.MaxInt64. + // decodeDeferredCheckpoints reads this count first and must reject it + // against the remaining-bytes bound before make([]DeferredCheckpoint, 0, + // count) can over-allocate. + f.Add([]byte{0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}) + + f.Fuzz(func(t *testing.T, data []byte) { + decoded, err := decodeDeferredCheckpoints(data) + if err != nil { + return + } + + // The per-entry codec is deliberately lenient (it skips unknown + // odd TLV types for forward-compat), so a re-encode is not + // byte-exact against an adversarial input. We instead assert a + // stable fixed point: encode the decoded value and confirm it + // re-decodes to an equal value. The first encode IS canonical, + // so the second round must be byte-stable. + out, err := encodeDeferredCheckpoints(decoded) + if err != nil { + t.Fatalf("re-encode: %v", err) + } + + redecoded, err := decodeDeferredCheckpoints(out) + if err != nil { + t.Fatalf("re-decode: %v", err) + } + + reencoded, err := encodeDeferredCheckpoints(redecoded) + if err != nil { + t.Fatalf("re-encode 2: %v", err) + } + + if !bytes.Equal(out, reencoded) { + t.Fatalf("deferred checkpoints not a stable fixed "+ + "point:\n got %x\nwant %x", reencoded, out) + } + }) +} diff --git a/unroll/deferred_checkpoint_codec.go b/unroll/deferred_checkpoint_codec.go index b4283353b..0f41c9c55 100644 --- a/unroll/deferred_checkpoint_codec.go +++ b/unroll/deferred_checkpoint_codec.go @@ -3,6 +3,7 @@ package unroll import ( "bytes" "fmt" + "io" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/lightningnetwork/lnd/tlv" @@ -107,6 +108,16 @@ func decodeDeferredCheckpoints(raw []byte) ([]DeferredCheckpoint, error) { err) } + // Bound the declared count against the bytes physically present + // before allocating. Each entry is at least a one-byte length prefix, + // so a count larger than the remaining bytes is a lie that would + // otherwise drive an unbounded (or panicking) make() from a tampered + // or truncated durable blob. + if count > uint64(reader.Len()) { + return nil, fmt.Errorf("deferred checkpoint count %d exceeds "+ + "%d remaining bytes", count, reader.Len()) + } + checkpoints := make([]DeferredCheckpoint, 0, count) for i := uint64(0); i < count; i++ { entryLen, err := tlv.ReadVarInt(reader, &scratch) @@ -115,8 +126,21 @@ func decodeDeferredCheckpoints(raw []byte) ([]DeferredCheckpoint, error) { "length: %w", i, err) } + // Bound each entry length against the bytes still available so + // a huge declared length cannot pre-allocate before we discover + // the truncation. + if entryLen > uint64(reader.Len()) { + return nil, fmt.Errorf("deferred checkpoint %d length "+ + "%d exceeds %d remaining bytes", i, entryLen, + reader.Len()) + } + + // Use io.ReadFull rather than reader.Read: a bare Read can + // return a short count without error, which would silently + // decode a truncated entry into a wrong-but-valid value + // (zero-padded tail) instead of failing closed. entry := make([]byte, entryLen) - if _, err := reader.Read(entry); err != nil { + if _, err := io.ReadFull(reader, entry); err != nil { return nil, fmt.Errorf("read deferred checkpoint %d "+ "body: %w", i, err) } @@ -160,7 +184,17 @@ func decodeDeferredCheckpointEntry(raw []byte) (DeferredCheckpoint, error) { err) } - if err := stream.Decode(bytes.NewReader(raw)); err != nil { + // Validate the inner entry framing before decoding. The txid record + // decodes through tlv DVarBytes, whose non-P2P path sizes + // make([]byte, length) directly from the declared length; without this + // guard a per-entry blob declaring a huge txid length panics or OOMs. + safeReader, err := safeTLVReader(raw) + if err != nil { + return DeferredCheckpoint{}, fmt.Errorf("decode entry "+ + "stream: %w", err) + } + + if err := stream.Decode(safeReader); err != nil { return DeferredCheckpoint{}, fmt.Errorf("decode entry "+ "stream: %w", err) } diff --git a/unroll/messages.go b/unroll/messages.go index cbac2d8bb..404d01277 100644 --- a/unroll/messages.go +++ b/unroll/messages.go @@ -228,7 +228,20 @@ func (m *StartUnrollRequest) Decode(r io.Reader) error { return fmt.Errorf("create stream: %w", err) } - parsed, err := stream.DecodeWithParsedTypes(r) + // Bound the record framing against bytes physically present before + // decoding: the lnd tlv non-P2P path sizes allocations from declared + // record lengths, so a hostile mailbox payload could otherwise panic + // or OOM via the policy-ref/kind DVarBytes records. + raw, err := io.ReadAll(r) + if err != nil { + return fmt.Errorf("read: %w", err) + } + safeReader, err := safeTLVReader(raw) + if err != nil { + return fmt.Errorf("decode: %w", err) + } + + parsed, err := stream.DecodeWithParsedTypes(safeReader) if err != nil { return fmt.Errorf("decode: %w", err) } @@ -299,7 +312,7 @@ func (m *ResumeUnrollRequest) Decode(r io.Reader) error { return fmt.Errorf("create stream: %w", err) } - if err := stream.Decode(r); err != nil { + if err := safeDecodeStream(stream, r); err != nil { return fmt.Errorf("decode: %w", err) } @@ -359,7 +372,7 @@ func (m *HeightObservedMsg) Decode(r io.Reader) error { return fmt.Errorf("create stream: %w", err) } - if err := stream.Decode(r); err != nil { + if err := safeDecodeStream(stream, r); err != nil { return fmt.Errorf("decode: %w", err) } @@ -439,7 +452,7 @@ func (m *TxConfirmedMsg) Decode(r io.Reader) error { return fmt.Errorf("create stream: %w", err) } - if err := stream.Decode(r); err != nil { + if err := safeDecodeStream(stream, r); err != nil { return fmt.Errorf("decode: %w", err) } @@ -510,7 +523,7 @@ func (m *TxFailedMsg) Decode(r io.Reader) error { return fmt.Errorf("create stream: %w", err) } - if err := stream.Decode(r); err != nil { + if err := safeDecodeStream(stream, r); err != nil { return fmt.Errorf("decode: %w", err) } @@ -595,7 +608,7 @@ func (m *SpendObservedMsg) Decode(r io.Reader) error { return fmt.Errorf("create stream: %w", err) } - if err := stream.Decode(r); err != nil { + if err := safeDecodeStream(stream, r); err != nil { return fmt.Errorf("decode: %w", err) } diff --git a/unroll/safetlv.go b/unroll/safetlv.go new file mode 100644 index 000000000..616c252f0 --- /dev/null +++ b/unroll/safetlv.go @@ -0,0 +1,88 @@ +package unroll + +import ( + "bytes" + "errors" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// ErrTLVRecordTooLarge signals that a TLV record declared a length larger than +// the bytes physically present in the input. The lnd tlv stream decoder's +// non-P2P path sizes make([]byte, length) (both for known DVarBytes records +// and the unknown-record discard buffer) directly from this attacker-controlled +// length without bounding it against the reader, so a tiny payload declaring a +// huge length panics (makeslice) or OOMs. The durable mailbox replays these +// message and checkpoint payloads from disk across rolling upgrades, so a +// tampered or truncated blob is a hostile input. We pre-validate framing to +// fail closed instead. +var ErrTLVRecordTooLarge = errors.New("tlv record length exceeds input") + +// safeTLVReader validates the (type, length) framing of an in-memory TLV blob +// against the bytes physically present, then returns a reader over the same +// bytes for the real decode. Rejecting an over-long record up front means the +// subsequent tlv.Stream.Decode can never reach the unbounded make([]byte, +// length) site with a length larger than the input. +func safeTLVReader(raw []byte) (*bytes.Reader, error) { + if err := validateTLVFraming(raw); err != nil { + return nil, err + } + + return bytes.NewReader(raw), nil +} + +// safeDecodeStream drains r into a bounded buffer and decodes it through the +// framing-validating reader. It exists for the mailbox message decoders, which +// receive an io.Reader (the outer codec frames each message) rather than a +// byte slice. Reading the message fully into memory is safe: the outer durable +// mailbox codec has already size-capped the per-message payload before handing +// it to us. +func safeDecodeStream(stream *tlv.Stream, r io.Reader) error { + raw, err := io.ReadAll(r) + if err != nil { + return err + } + + safeReader, err := safeTLVReader(raw) + if err != nil { + return err + } + + return stream.Decode(safeReader) +} + +// validateTLVFraming walks the record framing of an in-memory TLV blob and +// rejects any record whose declared length is larger than the remaining bytes. +func validateTLVFraming(raw []byte) error { + var scratch [8]byte + + reader := bytes.NewReader(raw) + for reader.Len() > 0 { + // A short read on the framing varints is a normal truncation + // the real decoder will also surface, so we stop validating and + // defer to its canonical error. + if _, err := tlv.ReadVarInt(reader, &scratch); err != nil { + return nil + } + + length, err := tlv.ReadVarInt(reader, &scratch) + if err != nil { + return nil + } + + if length > uint64(reader.Len()) { + return fmt.Errorf("%w: declared %d, %d remaining", + ErrTLVRecordTooLarge, length, reader.Len()) + } + + if _, err := reader.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return nil + } + } + + return nil +} diff --git a/unroll/snapshot.go b/unroll/snapshot.go index 4816a8a8f..978c779be 100644 --- a/unroll/snapshot.go +++ b/unroll/snapshot.go @@ -307,7 +307,17 @@ func decodeCheckpoint(raw []byte) (*actorCheckpoint, error) { return nil, fmt.Errorf("create checkpoint stream: %w", err) } - parsed, err := stream.DecodeWithParsedTypes(bytes.NewReader(raw)) + // Validate the record framing against the bytes physically present + // before decoding. The lnd tlv non-P2P decode path sizes allocations + // from the declared record length without bounding it, so a tampered + // checkpoint replayed from the durable mailbox could otherwise panic + // or OOM. + safeReader, err := safeTLVReader(raw) + if err != nil { + return nil, fmt.Errorf("decode checkpoint: %w", err) + } + + parsed, err := stream.DecodeWithParsedTypes(safeReader) if err != nil { return nil, fmt.Errorf("decode checkpoint: %w", err) } diff --git a/unroll/testdata/fuzz/FuzzDecodeDeferredCheckpoints/9281211b95853feb b/unroll/testdata/fuzz/FuzzDecodeDeferredCheckpoints/9281211b95853feb new file mode 100644 index 000000000..c3893668c --- /dev/null +++ b/unroll/testdata/fuzz/FuzzDecodeDeferredCheckpoints/9281211b95853feb @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte(" \x01\xff00000000000000000000000000000000000000") diff --git a/unroll/testdata/fuzz/FuzzDecodeDeferredCheckpoints/9b28e087f57a35f3 b/unroll/testdata/fuzz/FuzzDecodeDeferredCheckpoints/9b28e087f57a35f3 new file mode 100644 index 000000000..7fe415cac --- /dev/null +++ b/unroll/testdata/fuzz/FuzzDecodeDeferredCheckpoints/9b28e087f57a35f3 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("\x01(\x01 000000000000000000000000000000000\x04") diff --git a/unroll/testdata/fuzz/FuzzDecodeDeferredCheckpoints/ec1ef1497affeb07 b/unroll/testdata/fuzz/FuzzDecodeDeferredCheckpoints/ec1ef1497affeb07 new file mode 100644 index 000000000..59f62e740 --- /dev/null +++ b/unroll/testdata/fuzz/FuzzDecodeDeferredCheckpoints/ec1ef1497affeb07 @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("\x02(\x01 000000000000000000000000000000000\x040000(\x01 000000000000000000000000000000000\x040000") diff --git a/unrollplan/safetlv.go b/unrollplan/safetlv.go new file mode 100644 index 000000000..d9b482afb --- /dev/null +++ b/unrollplan/safetlv.go @@ -0,0 +1,68 @@ +package unrollplan + +import ( + "bytes" + "errors" + "fmt" + "io" + + "github.com/lightningnetwork/lnd/tlv" +) + +// ErrTLVRecordTooLarge signals that a TLV record declared a length larger than +// the bytes physically present in the input. The lnd tlv stream decoder's +// non-P2P path sizes make([]byte, length) (both for known DVarBytes records +// and the unknown-record discard buffer) directly from this attacker-controlled +// length without bounding it against the reader, so a tiny payload declaring a +// huge length panics (makeslice) or OOMs. The unrollplan State is persisted +// durably (nested inside the unroll actor checkpoint) and replayed across +// upgrades, so a tampered or truncated blob is a hostile input. We pre-validate +// framing to fail closed instead. +var ErrTLVRecordTooLarge = errors.New("tlv record length exceeds input") + +// safeTLVReader validates the (type, length) framing of an in-memory TLV blob +// against the bytes physically present, then returns a reader over the same +// bytes for the real decode. Rejecting an over-long record up front means the +// subsequent tlv.Stream.Decode can never reach the unbounded make([]byte, +// length) site with a length larger than the input. +func safeTLVReader(raw []byte) (*bytes.Reader, error) { + if err := validateTLVFraming(raw); err != nil { + return nil, err + } + + return bytes.NewReader(raw), nil +} + +// validateTLVFraming walks the record framing of an in-memory TLV blob and +// rejects any record whose declared length is larger than the remaining bytes. +func validateTLVFraming(raw []byte) error { + var scratch [8]byte + + reader := bytes.NewReader(raw) + for reader.Len() > 0 { + // A short read on the framing varints is a normal truncation + // the real decoder will also surface, so we stop validating and + // defer to its canonical error. + if _, err := tlv.ReadVarInt(reader, &scratch); err != nil { + return nil + } + + length, err := tlv.ReadVarInt(reader, &scratch) + if err != nil { + return nil + } + + if length > uint64(reader.Len()) { + return fmt.Errorf("%w: declared %d, %d remaining", + ErrTLVRecordTooLarge, length, reader.Len()) + } + + if _, err := reader.Seek( + int64(length), io.SeekCurrent, + ); err != nil { + return nil + } + } + + return nil +} diff --git a/unrollplan/state_codec.go b/unrollplan/state_codec.go index e803b0896..e3097277d 100644 --- a/unrollplan/state_codec.go +++ b/unrollplan/state_codec.go @@ -135,7 +135,16 @@ func DecodeState(raw []byte) (*State, error) { return nil, fmt.Errorf("create state stream: %w", err) } - parsed, err := stream.DecodeWithParsedTypes(bytes.NewReader(raw)) + // Validate the record framing against the bytes physically present + // before decoding. The lnd tlv non-P2P decode path sizes allocations + // from the declared record length without bounding it, so a tampered + // durable blob could otherwise panic or OOM. + safeReader, err := safeTLVReader(raw) + if err != nil { + return nil, fmt.Errorf("decode state: %w", err) + } + + parsed, err := stream.DecodeWithParsedTypes(safeReader) if err != nil { return nil, fmt.Errorf("decode state: %w", err) } @@ -303,7 +312,15 @@ func decodeSweepState(raw []byte) (SweepState, error) { return SweepState{}, err } - parsed, err := stream.DecodeWithParsedTypes(bytes.NewReader(raw)) + // As with the outer state stream, bound the nested sweep record + // framing before decoding to keep the unbounded allocation site + // unreachable from a tampered blob. + safeReader, err := safeTLVReader(raw) + if err != nil { + return SweepState{}, err + } + + parsed, err := stream.DecodeWithParsedTypes(safeReader) if err != nil { return SweepState{}, err } diff --git a/unrollplan/state_codec_fuzz_test.go b/unrollplan/state_codec_fuzz_test.go new file mode 100644 index 000000000..a45330a28 --- /dev/null +++ b/unrollplan/state_codec_fuzz_test.go @@ -0,0 +1,101 @@ +package unrollplan + +import ( + "testing" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/lightningnetwork/lnd/fn/v2" +) + +// FuzzDecodeState fuzzes the unrollplan State codec with attacker-controlled +// bytes. State is persisted durably (nested inside the unroll actor +// checkpoint) and replayed across upgrades, so DecodeState must never panic. +// The hash-list records and the nested sweep sub-stream both flow through tlv +// DVarBytes plus a hand-rolled big-endian count prefix. +func FuzzDecodeState(f *testing.F) { + seed := &State{ + ConfirmedTxids: []chainhash.Hash{{0x01}, {0x02}}, + InFlightTxids: []chainhash.Hash{{0x03}}, + TargetConfirmHeight: fn.Some[int32]( + 500, + ), + Sweep: SweepState{ + Status: SweepStatusBroadcasted, + Txid: fn.Some(chainhash.Hash{0x04}), + ConfirmHeight: fn.Some[int32](600), + }, + } + if b, err := EncodeState(seed); err == nil { + f.Add(b) + } + f.Add([]byte{}) + + // Canonical malicious payload: TLV type 11, then a BigSize-encoded + // length of math.MaxInt64. Without the safeTLVReader framing guard the + // decoder would size make([]byte, declaredLength) from this value and + // panic (makeslice) or OOM. Seeding it pins the guard's rejection path. + f.Add([]byte{ + 0x0b, 0xff, 0x7f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + }) + + f.Fuzz(func(t *testing.T, data []byte) { + state, err := DecodeState(data) + if err != nil { + return + } + + out, err := EncodeState(state) + if err != nil { + t.Fatalf("re-encode: %v", err) + } + + if _, err := DecodeState(out); err != nil { + t.Fatalf("re-decode: %v", err) + } + }) +} + +// FuzzDecodeHashList targets the hand-rolled hash-list codec directly: a 4-byte +// big-endian count followed by raw 32-byte hashes. The count guards a make() +// and a length check, so it is worth fuzzing in isolation. +func FuzzDecodeHashList(f *testing.F) { + if seed, err := encodeHashList([]chainhash.Hash{ + {0x01}, {0x02}, + }, "seed"); err == nil { + f.Add(seed) + } + f.Add([]byte{}) + + // Canonical malicious payload: a 4-byte big-endian count of + // 0xFFFFFFFF with no trailing hashes. decodeHashList must reject this + // via the exact count*HashSize length check before make([]chainhash.Hash, + // 0, count) can over-allocate. + f.Add([]byte{0xff, 0xff, 0xff, 0xff}) + + f.Fuzz(func(t *testing.T, data []byte) { + hashes, err := decodeHashList(data, "fuzz") + if err != nil { + return + } + + // encodeHashList canonicalizes by sorting, so the bytes are not + // expected to match a hand-crafted (possibly unsorted) input. + // We instead assert the encode succeeds and re-decodes to the + // same multiset, confirming the decoder produced a + // self-consistent value with no panic. + out, err := encodeHashList(hashes, "fuzz") + if err != nil { + t.Fatalf("re-encode: %v", err) + } + + redecoded, err := decodeHashList(out, "fuzz") + if err != nil { + t.Fatalf("re-decode: %v", err) + } + + if len(redecoded) != len(hashes) { + t.Fatalf("hash count drift: got %d want %d", + len(redecoded), len(hashes)) + } + }) +} diff --git a/unrollplan/testdata/fuzz/FuzzDecodeHashList/bbabc3ba7d43a4ce b/unrollplan/testdata/fuzz/FuzzDecodeHashList/bbabc3ba7d43a4ce new file mode 100644 index 000000000..787e71773 --- /dev/null +++ b/unrollplan/testdata/fuzz/FuzzDecodeHashList/bbabc3ba7d43a4ce @@ -0,0 +1,2 @@ +go test fuzz v1 +[]byte("\x00\x00\x00\x02000000000000000000000000000000000000000000000000000000000000000 ")