From 3dc3f4b20d7ee6798b08399ae5a16afd2dbf29a1 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 14 Aug 2026 11:14:05 -0700 Subject: [PATCH 01/38] test: add TDS response fuzz harness and end-to-end target Add token_fuzz_test.go with reusable helpers to frame arbitrary token streams into one or more packReply TDS packets (with configurable fragmentation), build a tdsSession reading from the framed bytes, and run processSingleResponse while fully draining its buffered token channel. Add FuzzProcessSingleResponse seeded with valid synthetic responses (DONE, COLMETADATA/ROW, NBCROW, multiple result sets, ERROR, INFO, RETURNSTATUS, DONEPROC/DONEINPROC, ENVCHANGE, TABNAME/COLINFO/ORDER) and malformed inputs (unknown token, truncated token, trailing garbage). Add TestProcessSingleResponsePacketBoundary asserting the parser's token sequence is independent of packet fragmentation for the valid seeds. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- token_fuzz_test.go | 315 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 315 insertions(+) create mode 100644 token_fuzz_test.go diff --git a/token_fuzz_test.go b/token_fuzz_test.go new file mode 100644 index 000000000..4a551dc92 --- /dev/null +++ b/token_fuzz_test.go @@ -0,0 +1,315 @@ +package mssql + +import ( + "bytes" + "context" + "encoding/binary" + "fmt" + "reflect" + "testing" +) + +// fuzzPacketSize is the TDS buffer size used by the response fuzz harness. +// It must be large enough to hold the biggest single packet the framing +// helper can produce. newTdsBuffer allocates a 64 KiB backing buffer and +// splits it in half, so the read buffer is 32 KiB; keep packets within that. +const fuzzPacketSize = 1 << 15 // 32768 + +// fuzzMaxPacketPayload is the largest token-stream payload that fits in a +// single framed packet given fuzzPacketSize and the 8 byte TDS header. +const fuzzMaxPacketPayload = fuzzPacketSize - 8 + +// fuzzTransport serves a fixed byte stream as the server side of a TDS +// connection. Writes are discarded because the response parser never replies +// on the happy path (attention/cancel writes are irrelevant to parsing here). +type fuzzTransport struct { + r *bytes.Reader +} + +func (t *fuzzTransport) Read(p []byte) (int, error) { return t.r.Read(p) } +func (t *fuzzTransport) Write(p []byte) (int, error) { return len(p), nil } +func (t *fuzzTransport) Close() error { return nil } + +// frameReplyPackets wraps an arbitrary token stream into one or more packReply +// TDS packets. frag selects how many packets the logical stream is split into +// (1..8), letting the same stream be fragmented at different byte boundaries so +// the parser is exercised across packet seams. Every non-final packet clears +// the final status bit; only the last packet sets it. +// +// It returns ok=false when the stream cannot be framed within the uint16 packet +// size limit (which, given the input bound in the fuzz body, never happens but +// is guarded defensively). +func frameReplyPackets(stream []byte, frag byte) (framed []byte, ok bool) { + const headerLen = 8 + + numFrags := 1 + int(frag)%8 + chunk := (len(stream) + numFrags - 1) / numFrags + if chunk < 1 { + chunk = 1 + } + if chunk > fuzzMaxPacketPayload { + chunk = fuzzMaxPacketPayload + } + + var out []byte + var seq byte = 1 + off := 0 + for { + end := off + chunk + if end >= len(stream) { + end = len(stream) + } + final := end == len(stream) + + size := headerLen + (end - off) + if size > int(^uint16(0)) { + return nil, false + } + + hdr := make([]byte, headerLen) + hdr[0] = byte(packReply) + if final { + hdr[1] = 0x01 // final packet + } + binary.BigEndian.PutUint16(hdr[2:4], uint16(size)) + hdr[6] = seq + out = append(out, hdr...) + out = append(out, stream[off:end]...) + + seq++ + off = end + if final { + break + } + } + return out, true +} + +// newFuzzSession builds a *tdsSession whose buffer reads from the given framed +// TDS packet bytes. logFlags stays zero so the (nil) logger is never invoked. +func newFuzzSession(framed []byte) *tdsSession { + transport := &fuzzTransport{r: bytes.NewReader(framed)} + return &tdsSession{ + buf: newTdsBuffer(fuzzPacketSize, transport), + } +} + +// drainSingleResponse runs processSingleResponse against a framed stream and +// fully drains the token channel so the reader goroutine never blocks on the +// size-5 buffered channel. It returns the ordered list of token Go types that +// were produced (used for the packet-boundary determinism invariant) and +// whether any error/panic token was emitted. processSingleResponse installs its +// own recover(), so a malformed stream surfaces as an error token here rather +// than crashing the harness. +func drainSingleResponse(stream []byte, frag byte) (tokenTypes []string, sawError bool, framed bool) { + packets, ok := frameReplyPackets(stream, frag) + if !ok { + return nil, false, false + } + sess := newFuzzSession(packets) + defer sess.buf.bufClose() + + ch := make(chan tokenStruct, 5) + go processSingleResponse(context.Background(), sess, ch, outputs{}) + + for tok := range ch { + tokenTypes = append(tokenTypes, fmt.Sprintf("%T", tok)) + if _, isErr := tok.(error); isErr { + sawError = true + } + } + return tokenTypes, sawError, true +} + +// --- synthetic token-stream builders ------------------------------------- + +// buildDone returns a DONE/DONEPROC/DONEINPROC token body (status, curcmd, +// rowcount). The caller supplies the leading token byte. +func doneBody(status uint16) []byte { + b := make([]byte, 1+2+2+8) + // b[0] is the token byte, filled in by the caller via prepend. + binary.LittleEndian.PutUint16(b[1:3], status) + // curcmd and rowcount left as zero + return b +} + +func doneToken(tok token, status uint16) []byte { + b := doneBody(status) + b[0] = byte(tok) + return b +} + +// colMetadataInt4 returns a COLMETADATA token for a single, unnamed int4 column. +func colMetadataInt4() []byte { + return []byte{ + byte(tokenColMetadata), + 0x01, 0x00, // count = 1 + 0x00, 0x00, 0x00, 0x00, // UserType + 0x00, 0x00, // Flags + typeInt4, // TypeId (fixed length, no extra type info) + 0x00, // ColName BVarChar length = 0 + } +} + +// rowInt4 returns a ROW token carrying one int4 value. +func rowInt4(v int32) []byte { + b := []byte{byte(tokenRow), 0, 0, 0, 0} + binary.LittleEndian.PutUint32(b[1:5], uint32(v)) + return b +} + +// nbcRowNull returns an NBCROW token whose single column is NULL (bit set), +// so no column data follows. +func nbcRowNull() []byte { + return []byte{byte(tokenNbcRow), 0x01} +} + +// infoLikeToken builds an ERROR/INFO token body (they share a layout). +func infoLikeToken(tok token) []byte { + return []byte{ + byte(tok), + 0x00, 0x00, // Length (ignored) + 0x00, 0x00, 0x00, 0x00, // Number + 0x01, // State + 0x01, // Class + 0x00, 0x00, // Message UsVarChar length = 0 + 0x00, // ServerName BVarChar length = 0 + 0x00, // ProcName BVarChar length = 0 + 0x00, 0x00, 0x00, 0x00, // LineNo + } +} + +// envChangeDatabase builds an ENVCHANGE token announcing a database change. +func envChangeDatabase() []byte { + // payload: type(1) + new BVarChar("x") + old BVarChar("") + payload := []byte{ + envTypDatabase, + 0x01, 'x', 0x00, // new value: len 1, UCS2 'x' + 0x00, // old value: len 0 + } + b := []byte{byte(tokenEnvChange), 0x00, 0x00} + binary.LittleEndian.PutUint16(b[1:3], uint16(len(payload))) + return append(b, payload...) +} + +func returnStatusToken(v int32) []byte { + b := []byte{byte(tokenReturnStatus), 0, 0, 0, 0} + binary.LittleEndian.PutUint32(b[1:5], uint32(v)) + return b +} + +func concat(parts ...[]byte) []byte { + var out []byte + for _, p := range parts { + out = append(out, p...) + } + return out +} + +// fuzzResponseSeeds returns complete, valid synthetic response streams plus a +// few deliberately malformed ones. These double as the packet-boundary +// determinism corpus (see TestProcessSingleResponsePacketBoundary). +func fuzzResponseSeeds() [][]byte { + return [][]byte{ + // empty result set + DONE + doneToken(tokenDone, doneFinal), + // COLMETADATA -> ROW -> DONE + concat(colMetadataInt4(), rowInt4(42), doneToken(tokenDone, doneFinal)), + // COLMETADATA -> NBCROW(null) -> DONE + concat(colMetadataInt4(), nbcRowNull(), doneToken(tokenDone, doneFinal)), + // multiple result sets: DONE(doneMore) then final DONE + concat(doneToken(tokenDone, doneMore), doneToken(tokenDone, doneFinal)), + // ERROR -> DONE + concat(infoLikeToken(tokenError), doneToken(tokenDone, doneFinal)), + // INFO -> DONE + concat(infoLikeToken(tokenInfo), doneToken(tokenDone, doneFinal)), + // RETURNSTATUS -> DONE + concat(returnStatusToken(0), doneToken(tokenDone, doneFinal)), + // DONEPROC (final) + doneToken(tokenDoneProc, doneFinal), + // DONEINPROC (final) + doneToken(tokenDoneInProc, doneFinal), + // ENVCHANGE(database) -> DONE + concat(envChangeDatabase(), doneToken(tokenDone, doneFinal)), + // TABNAME + COLINFO + ORDER -> DONE + concat( + []byte{byte(tokenTabName), 0x00, 0x00}, + []byte{byte(tokenColInfo), 0x00, 0x00}, + []byte{byte(tokenOrder), 0x00, 0x00}, + doneToken(tokenDone, doneFinal), + ), + // unknown token id -> handled via recover into an error token + {0x00}, + // truncated DONE token (missing bytes) -> recovered error token + {byte(tokenDone), 0x00}, + // valid final DONE followed by trailing garbage (ignored after return) + concat(doneToken(tokenDone, doneFinal), []byte{0xDE, 0xAD, 0xBE, 0xEF}), + } +} + +// TestProcessSingleResponsePacketBoundary asserts that, for known-valid seeds, +// the sequence of token types produced by the parser is independent of how the +// stream is fragmented across TDS packets. +func TestProcessSingleResponsePacketBoundary(t *testing.T) { + for i, seed := range fuzzResponseSeeds() { + seed := seed + t.Run(fmt.Sprintf("seed_%d", i), func(t *testing.T) { + single, _, ok := drainSingleResponse(seed, 0) // one packet + if !ok { + t.Fatal("failed to frame seed as a single packet") + } + for _, frag := range []byte{1, 3, 7, 255} { + fragmented, _, ok := drainSingleResponse(seed, frag) + if !ok { + t.Fatalf("failed to frame seed with frag=%d", frag) + } + if !reflect.DeepEqual(single, fragmented) { + t.Fatalf("token sequence differs by packet boundary (frag=%d):\n single=%v\n frag =%v", + frag, single, fragmented) + } + } + }) + } +} + +// FuzzProcessSingleResponse feeds arbitrary token streams to the core TDS +// response parser (processSingleResponse) after framing them into one or more +// packReply packets. The parser is expected to either parse the stream or +// convert a malformed stream into an error token via its internal recover(); +// the invariant this fuzz target enforces is that it must NEVER panic out of +// the harness regardless of input, and the reader goroutine must always +// terminate (the drain helper guarantees the channel is emptied). +// +// NOTE (issue #418 hardening): the seed corpus below runs clean under +// `go test` and is what CI relies on. An extended `go test -fuzz` run of this +// target currently surfaces a pre-existing robustness gap: several TDS token +// parsers allocate buffers sized directly from attacker-controlled length +// prefixes before validating them against the available data (for example +// parseFedAuthInfo's make([]byte, size-offset) with a uint32 size, the +// variable-length column readers in types.go, and parseColMetadata72's column +// count). Under sustained fuzzing these unbounded allocations accumulate and +// exhaust memory. Bounding them is tracked as follow-up work in the +// fuzz-coverage stack and is intentionally out of scope for this +// test-infrastructure layer. +func FuzzProcessSingleResponse(f *testing.F) { + for _, seed := range fuzzResponseSeeds() { + f.Add(seed, byte(0)) + f.Add(seed, byte(3)) + } + // A couple of raw single-token seeds for extra coverage. + f.Add([]byte{byte(tokenColMetadata)}, byte(0)) + f.Add([]byte{}, byte(0)) + + f.Fuzz(func(t *testing.T, stream []byte, frag byte) { + // Bound input size to keep framing and allocations reasonable. A TDS + // packet length is a uint16, and the read buffer is 32 KiB, so very + // large inputs would either fail to frame or blow the buffer. + if len(stream) > 64*1024 { + t.Skip() + } + // The invariant: this must return normally (no panic escaping the + // parser's recover, no goroutine leak/deadlock). The returned values + // are intentionally unused beyond confirming completion. + _, _, _ = drainSingleResponse(stream, frag) + }) +} From 8a474c6b3ee32686de3ece8ad7bb3efa0377186b Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 14 Aug 2026 11:32:38 -0700 Subject: [PATCH 02/38] fix: bound TDS response allocations to prevent OOM from malformed streams Several TDS token parsers allocated buffers and slices sized directly from attacker-controlled length prefixes before validating them against the available data, so a malformed server response could drive an unbounded allocation and OOM the client (a DoS). The stream recover() does not catch OOM because it is uncontrolled allocation, not a panic. Bound the wire-derived allocation sizes and reject underflows via badStreamPanic so a malformed stream fails cleanly as an error token: - parseFedAuthInfo: cap the token size and reject an option count that cannot fit within it (fixes the EE 00*8 ~4GB underflow repro). - readLongLenType: reject negative/oversized TEXT/NTEXT/IMAGE lengths. - readVariantTypeWithEncoding: reject underflowed/oversized data lengths. Add regression unit tests plus permanent fuzz seeds for the crafted inputs. Fixes #420 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- token.go | 16 ++++ token_alloc_regression_test.go | 165 +++++++++++++++++++++++++++++++++ token_fuzz_test.go | 30 +++--- types.go | 12 +++ 4 files changed, 212 insertions(+), 11 deletions(-) create mode 100644 token_alloc_regression_test.go diff --git a/token.go b/token.go index 834cb2653..36384ad66 100644 --- a/token.go +++ b/token.go @@ -500,8 +500,18 @@ type fedAuthInfoOpt struct { dataLength, dataOffset uint32 } +// _MAX_FEDAUTHINFO_LEN bounds the total FEDAUTHINFO token size. The token only +// carries a small STSURL and SPN, so any larger advertised size is a malformed +// or hostile stream rather than something we should allocate for. The cap keeps +// an attacker-controlled length prefix from driving an unbounded allocation +// (OOM DoS, issue #420); a violation fails the stream as a StreamError. +const _MAX_FEDAUTHINFO_LEN = 1 << 20 + func parseFedAuthInfo(r *tdsBuffer) fedAuthInfoStruct { size := r.uint32() + if size > _MAX_FEDAUTHINFO_LEN { + badStreamPanicf("federated authentication info size %d exceeds maximum of %d bytes", size, _MAX_FEDAUTHINFO_LEN) + } var STSURL, SPN string var err error @@ -510,6 +520,12 @@ func parseFedAuthInfo(r *tdsBuffer) fedAuthInfoStruct { // then a four byte offset and a four byte length. count := r.uint32() offset := uint32(4) + // The option headers (9 bytes each) plus the trailing data must all fit + // within the advertised token size. Reject a count that cannot fit before + // allocating, so a bogus count cannot pre-allocate gigabytes of options. + if uint64(count)*9+uint64(offset) > uint64(size) { + badStreamPanicf("federated authentication info advertised %d options that do not fit in %d bytes", count, size) + } opts := make([]fedAuthInfoOpt, count) for i := uint32(0); i < count; i++ { diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go new file mode 100644 index 000000000..90270e253 --- /dev/null +++ b/token_alloc_regression_test.go @@ -0,0 +1,165 @@ +package mssql + +import ( + "encoding/binary" + "testing" + + "github.com/microsoft/go-mssqldb/msdsn" + "github.com/stretchr/testify/assert" +) + +// bufFromBytes builds a *tdsBuffer that serves stream as a single, final packet +// payload. It mirrors the helper style in types_test.go so a token parser can be +// exercised directly without a live connection. +func bufFromBytes(stream []byte) *tdsBuffer { + size := len(stream) + if size == 0 { + size = 1 + } + buf := newTdsBuffer(uint16(1<<15), nil) + copy(buf.rbuf[:len(stream)], stream) + buf.rpos = 0 + buf.rsize = len(stream) + buf.final = true + return buf +} + +// recoverErr runs fn and returns any panic value coerced to an error. Token +// parsers signal a malformed stream by panicking (badStreamPanic/badStreamPanicf) +// which processSingleResponse recovers into an error token. +func recoverErr(fn func()) (err error) { + defer func() { + if r := recover(); r != nil { + if e, ok := r.(error); ok { + err = e + } else { + err = assert.AnError + } + } + }() + fn() + return nil +} + +// TestParseFedAuthInfo_MalformedAllocations is a regression test for issue #420: +// parseFedAuthInfo allocated buffers/slices sized directly from attacker +// controlled length prefixes before validating them, so a crafted FEDAUTHINFO +// token drove an unbounded allocation (OOM DoS). The repro from the issue is the +// nine bytes `EE 00 00 00 00 00 00 00 00`; here we feed just the token body +// (the EE token byte is consumed by processSingleResponse before the parser). +func TestParseFedAuthInfo_MalformedAllocations(t *testing.T) { + le := binary.LittleEndian + u32 := func(v uint32) []byte { + b := make([]byte, 4) + le.PutUint32(b, v) + return b + } + + cases := map[string]struct { + stream []byte + wantSub string + }{ + // size=0, count=0: the original make([]byte, size-offset) underflowed to + // ~4 GB. The option-count check now rejects it first. + "underflow size zero": { + stream: append(u32(0), u32(0)...), + wantSub: "do not fit", + }, + // A small size with a huge option count previously pre-allocated a giant + // opts slice before any bounds check. + "bogus option count": { + stream: append(u32(8), u32(0xFFFFFFFF)...), + wantSub: "do not fit", + }, + // A size larger than any real FEDAUTHINFO token is rejected outright. + "oversized token": { + stream: append(u32(_MAX_FEDAUTHINFO_LEN+1), u32(0)...), + wantSub: "exceeds maximum", + }, + } + + for name, tc := range cases { + tc := tc + t.Run(name, func(t *testing.T) { + err := recoverErr(func() { parseFedAuthInfo(bufFromBytes(tc.stream)) }) + if err == nil { + t.Fatalf("expected a stream error, got none") + } + assert.Contains(t, err.Error(), tc.wantSub) + }) + } +} + +// TestReadLongLenType_MalformedLengthPanics is a regression test for issue #420: +// readLongLenType used the untrusted int32 length as the buffer size, so a +// negative length aborted with an out-of-range make() and a huge one allocated +// gigabytes. Both must fail the stream cleanly instead. +func TestReadLongLenType_MalformedLengthPanics(t *testing.T) { + build := func(size int32) []byte { + // textptrsize=1, textptr(1 byte), timestamp(8 bytes), size(int32) + b := []byte{0x01, 0x00, 0, 0, 0, 0, 0, 0, 0, 0} + var s [4]byte + binary.LittleEndian.PutUint32(s[:], uint32(size)) + return append(b, s[:]...) + } + + sizes := map[string]int32{ + "negative length": -2, + } + for name, size := range sizes { + size := size + t.Run(name, func(t *testing.T) { + ti := typeInfo{TypeId: typeImage} + err := recoverErr(func() { + readLongLenType(&ti, bufFromBytes(build(size)), nil, msdsn.EncodeParameters{}) + }) + if err == nil { + t.Fatalf("expected a stream error, got none") + } + assert.Contains(t, err.Error(), "maximum LOB size") + }) + } +} + +// TestReadVariantType_UnderflowPanics is a regression test for issue #420: +// readVariantTypeWithEncoding allocated make([]byte, size-2-propbytes) which +// underflowed to a huge value when propbytes exceeded size-2. +func TestReadVariantType_UnderflowPanics(t *testing.T) { + // size=3, vartype=typeGuid, propbytes=250 -> 3-2-250 = -249 + stream := []byte{0x03, 0x00, 0x00, 0x00, typeGuid, 0xFA} + ti := typeInfo{} + err := recoverErr(func() { + readVariantTypeWithEncoding(&ti, bufFromBytes(stream), nil, msdsn.EncodeParameters{}) + }) + if err == nil { + t.Fatalf("expected a stream error, got none") + } + assert.Contains(t, err.Error(), "sql_variant data length") +} + +// TestProcessSingleResponse_MalformedNoOOM feeds crafted malformed token streams +// (framed as reply packets) through the full response parser and asserts each is +// turned into an error token rather than hanging or exhausting memory. These are +// the end-to-end forms of the issue #420 repros. +func TestProcessSingleResponse_MalformedNoOOM(t *testing.T) { + streams := map[string][]byte{ + // FEDAUTHINFO underflow repro from the issue: EE 00 00 00 00 00 00 00 00 + "fedauth underflow": {byte(tokenFedAuthInfo), 0, 0, 0, 0, 0, 0, 0, 0}, + // COLMETADATA with a bogus-huge column count (0xFFFE, not the 0xFFFF + // "no metadata" sentinel) followed by no column data. + "colmetadata bogus count": {byte(tokenColMetadata), 0xFE, 0xFF}, + } + + for name, stream := range streams { + stream := stream + t.Run(name, func(t *testing.T) { + _, sawError, framed := drainSingleResponse(stream, 0) + if !framed { + t.Fatalf("failed to frame stream") + } + if !sawError { + t.Fatalf("expected an error token for malformed stream") + } + }) + } +} diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 4a551dc92..e9d3e4111 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -280,17 +280,15 @@ func TestProcessSingleResponsePacketBoundary(t *testing.T) { // the harness regardless of input, and the reader goroutine must always // terminate (the drain helper guarantees the channel is emptied). // -// NOTE (issue #418 hardening): the seed corpus below runs clean under -// `go test` and is what CI relies on. An extended `go test -fuzz` run of this -// target currently surfaces a pre-existing robustness gap: several TDS token -// parsers allocate buffers sized directly from attacker-controlled length -// prefixes before validating them against the available data (for example -// parseFedAuthInfo's make([]byte, size-offset) with a uint32 size, the -// variable-length column readers in types.go, and parseColMetadata72's column -// count). Under sustained fuzzing these unbounded allocations accumulate and -// exhaust memory. Bounding them is tracked as follow-up work in the -// fuzz-coverage stack and is intentionally out of scope for this -// test-infrastructure layer. +// NOTE (issue #420 hardening): several TDS token parsers previously allocated +// buffers sized directly from attacker-controlled length prefixes before +// validating them against the available data (for example parseFedAuthInfo's +// make([]byte, size-offset) with a uint32 size, the variable-length column +// readers in types.go, and parseColMetadata72's column count). Under sustained +// fuzzing these unbounded allocations exhausted memory. Those sizes/counts are +// now bounded and underflows rejected via badStreamPanic, so a malformed stream +// fails cleanly as an error token instead of OOMing; the crafted repros are +// pinned as seeds above. func FuzzProcessSingleResponse(f *testing.F) { for _, seed := range fuzzResponseSeeds() { f.Add(seed, byte(0)) @@ -300,6 +298,16 @@ func FuzzProcessSingleResponse(f *testing.F) { f.Add([]byte{byte(tokenColMetadata)}, byte(0)) f.Add([]byte{}, byte(0)) + // Regression seeds for issue #420: token streams whose length prefixes drove + // unbounded allocations before they were bounded. They must now parse into an + // error token without OOMing. + // FEDAUTHINFO underflow repro from the issue (EE 00*8): size=0,count=0. + f.Add([]byte{byte(tokenFedAuthInfo), 0, 0, 0, 0, 0, 0, 0, 0}, byte(0)) + // FEDAUTHINFO with a bogus-huge option count. + f.Add([]byte{byte(tokenFedAuthInfo), 8, 0, 0, 0, 0xFF, 0xFF, 0xFF, 0xFF}, byte(0)) + // COLMETADATA with a bogus-huge column count (0xFFFE) and no column data. + f.Add([]byte{byte(tokenColMetadata), 0xFE, 0xFF}, byte(0)) + f.Fuzz(func(t *testing.T, stream []byte, frag byte) { // Bound input size to keep framing and allocations reasonable. A TDS // packet length is a uint16, and the read buffer is 32 KiB, so very diff --git a/types.go b/types.go index cc817715a..9521ebe56 100644 --- a/types.go +++ b/types.go @@ -574,6 +574,11 @@ func readLongLenType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msd if size == -1 { return nil } + // The advertised size is attacker-controlled; reject anything a real server + // cannot produce before using it as an allocation size (OOM DoS, issue #420). + if size < 0 || int64(size) > _MAX_PLP_LEN { + badStreamPanicf("TEXT/NTEXT/IMAGE length %d exceeds the maximum LOB size of %d bytes", size, int64(_MAX_PLP_LEN)) + } buf := make([]byte, size) r.ReadFull(buf) switch ti.TypeId { @@ -655,6 +660,13 @@ func readVariantTypeWithEncoding(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, } vartype := r.byte() propbytes := int32(r.byte()) + // size-2-propbytes is the trailing data length and is used below as an + // allocation size. It is derived from an attacker-controlled size prefix, + // so reject an underflowed (negative) or implausibly large value before any + // make() to avoid an OOM DoS (issue #420). + if datalen := size - 2 - propbytes; datalen < 0 || int64(datalen) > _MAX_PLP_LEN { + badStreamPanicf("sql_variant data length %d is invalid", datalen) + } switch vartype { case typeGuid: buf := make([]byte, size-2-propbytes) From 062a0092de64fd401d94ec8b1e8fc7dbc80d7e0b Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 14 Aug 2026 14:46:58 -0700 Subject: [PATCH 03/38] test: remove ineffectual assignment in bufFromBytes helper Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- token_alloc_regression_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index 90270e253..537509912 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -12,10 +12,6 @@ import ( // payload. It mirrors the helper style in types_test.go so a token parser can be // exercised directly without a live connection. func bufFromBytes(stream []byte) *tdsBuffer { - size := len(stream) - if size == 0 { - size = 1 - } buf := newTdsBuffer(uint16(1<<15), nil) copy(buf.rbuf[:len(stream)], stream) buf.rpos = 0 From bea21c14d200718db780ba26742d18b9b8621b21 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 14 Aug 2026 14:48:32 -0700 Subject: [PATCH 04/38] test: address review feedback on TDS fuzz harness - Correct frameReplyPackets doc: frag is a target fragment count (1..8), streams exceeding the per-packet limit split into additional packets. - Fix stale doneBody helper comment (was buildDone). - Set a spec-faithful Length field on ERROR/INFO seed tokens. - Restrict the packet-boundary determinism test to known-valid seeds by splitting seeds into validResponseSeeds/malformedResponseSeeds. - Skip per-token fmt.Sprintf allocations in the fuzz hot path via a collect flag on drainSingleResponse. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 81 ++++++++++++++++++++++++++++++---------------- 1 file changed, 54 insertions(+), 27 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 4a551dc92..46e48d6ce 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -31,10 +31,12 @@ func (t *fuzzTransport) Write(p []byte) (int, error) { return len(p), nil } func (t *fuzzTransport) Close() error { return nil } // frameReplyPackets wraps an arbitrary token stream into one or more packReply -// TDS packets. frag selects how many packets the logical stream is split into -// (1..8), letting the same stream be fragmented at different byte boundaries so -// the parser is exercised across packet seams. Every non-final packet clears -// the final status bit; only the last packet sets it. +// TDS packets. frag selects a target fragment count in the range 1..8, so the +// same stream is fragmented at different byte boundaries and the parser is +// exercised across packet seams. The target is only a lower bound: any fragment +// larger than fuzzMaxPacketPayload is split further, so a stream longer than +// 8*fuzzMaxPacketPayload can produce more than 8 packets. Every non-final packet +// clears the final status bit; only the last packet sets it. // // It returns ok=false when the stream cannot be framed within the uint16 packet // size limit (which, given the input bound in the fuzz body, never happens but @@ -96,12 +98,14 @@ func newFuzzSession(framed []byte) *tdsSession { // drainSingleResponse runs processSingleResponse against a framed stream and // fully drains the token channel so the reader goroutine never blocks on the -// size-5 buffered channel. It returns the ordered list of token Go types that -// were produced (used for the packet-boundary determinism invariant) and -// whether any error/panic token was emitted. processSingleResponse installs its -// own recover(), so a malformed stream surfaces as an error token here rather -// than crashing the harness. -func drainSingleResponse(stream []byte, frag byte) (tokenTypes []string, sawError bool, framed bool) { +// size-5 buffered channel. When collect is true it returns the ordered list of +// token Go types that were produced (used for the packet-boundary determinism +// invariant); the fuzz target passes collect=false to avoid the per-token +// fmt.Sprintf allocations in the hot path. It also reports whether any +// error/panic token was emitted. processSingleResponse installs its own +// recover(), so a malformed stream surfaces as an error token here rather than +// crashing the harness. +func drainSingleResponse(stream []byte, frag byte, collect bool) (tokenTypes []string, sawError bool, framed bool) { packets, ok := frameReplyPackets(stream, frag) if !ok { return nil, false, false @@ -113,7 +117,9 @@ func drainSingleResponse(stream []byte, frag byte) (tokenTypes []string, sawErro go processSingleResponse(context.Background(), sess, ch, outputs{}) for tok := range ch { - tokenTypes = append(tokenTypes, fmt.Sprintf("%T", tok)) + if collect { + tokenTypes = append(tokenTypes, fmt.Sprintf("%T", tok)) + } if _, isErr := tok.(error); isErr { sawError = true } @@ -123,7 +129,7 @@ func drainSingleResponse(stream []byte, frag byte) (tokenTypes []string, sawErro // --- synthetic token-stream builders ------------------------------------- -// buildDone returns a DONE/DONEPROC/DONEINPROC token body (status, curcmd, +// doneBody returns a DONE/DONEPROC/DONEINPROC token body (status, curcmd, // rowcount). The caller supplies the leading token byte. func doneBody(status uint16) []byte { b := make([]byte, 1+2+2+8) @@ -164,11 +170,11 @@ func nbcRowNull() []byte { return []byte{byte(tokenNbcRow), 0x01} } -// infoLikeToken builds an ERROR/INFO token body (they share a layout). +// infoLikeToken builds an ERROR/INFO token body (they share a layout). The +// Length field is set to the true byte length of the token data that follows +// it, so the seed is spec-faithful even though the current parser ignores it. func infoLikeToken(tok token) []byte { - return []byte{ - byte(tok), - 0x00, 0x00, // Length (ignored) + body := []byte{ 0x00, 0x00, 0x00, 0x00, // Number 0x01, // State 0x01, // Class @@ -177,6 +183,9 @@ func infoLikeToken(tok token) []byte { 0x00, // ProcName BVarChar length = 0 0x00, 0x00, 0x00, 0x00, // LineNo } + out := []byte{byte(tok), 0x00, 0x00} // token id + Length placeholder + binary.LittleEndian.PutUint16(out[1:3], uint16(len(body))) + return append(out, body...) } // envChangeDatabase builds an ENVCHANGE token announcing a database change. @@ -206,10 +215,10 @@ func concat(parts ...[]byte) []byte { return out } -// fuzzResponseSeeds returns complete, valid synthetic response streams plus a -// few deliberately malformed ones. These double as the packet-boundary -// determinism corpus (see TestProcessSingleResponsePacketBoundary). -func fuzzResponseSeeds() [][]byte { +// validResponseSeeds returns complete, well-formed synthetic response streams. +// Each parses to a deterministic token sequence, so these double as the +// packet-boundary determinism corpus (see TestProcessSingleResponsePacketBoundary). +func validResponseSeeds() [][]byte { return [][]byte{ // empty result set + DONE doneToken(tokenDone, doneFinal), @@ -238,28 +247,45 @@ func fuzzResponseSeeds() [][]byte { []byte{byte(tokenOrder), 0x00, 0x00}, doneToken(tokenDone, doneFinal), ), + // valid final DONE followed by trailing garbage: the parser returns on + // the final DONE before reading the garbage, so this still parses cleanly + // and deterministically regardless of packet boundaries. + concat(doneToken(tokenDone, doneFinal), []byte{0xDE, 0xAD, 0xBE, 0xEF}), + } +} + +// malformedResponseSeeds returns deliberately broken streams. Each is expected +// to be converted into an error token by processSingleResponse's recover(). +// These are NOT part of the packet-boundary determinism corpus because where the +// parser trips can legitimately depend on how the bytes are split into packets. +func malformedResponseSeeds() [][]byte { + return [][]byte{ // unknown token id -> handled via recover into an error token {0x00}, // truncated DONE token (missing bytes) -> recovered error token {byte(tokenDone), 0x00}, - // valid final DONE followed by trailing garbage (ignored after return) - concat(doneToken(tokenDone, doneFinal), []byte{0xDE, 0xAD, 0xBE, 0xEF}), } } +// fuzzResponseSeeds returns every synthetic seed (valid and malformed) for use +// as the fuzz corpus. +func fuzzResponseSeeds() [][]byte { + return append(validResponseSeeds(), malformedResponseSeeds()...) +} + // TestProcessSingleResponsePacketBoundary asserts that, for known-valid seeds, // the sequence of token types produced by the parser is independent of how the // stream is fragmented across TDS packets. func TestProcessSingleResponsePacketBoundary(t *testing.T) { - for i, seed := range fuzzResponseSeeds() { + for i, seed := range validResponseSeeds() { seed := seed t.Run(fmt.Sprintf("seed_%d", i), func(t *testing.T) { - single, _, ok := drainSingleResponse(seed, 0) // one packet + single, _, ok := drainSingleResponse(seed, 0, true) // one packet if !ok { t.Fatal("failed to frame seed as a single packet") } for _, frag := range []byte{1, 3, 7, 255} { - fragmented, _, ok := drainSingleResponse(seed, frag) + fragmented, _, ok := drainSingleResponse(seed, frag, true) if !ok { t.Fatalf("failed to frame seed with frag=%d", frag) } @@ -309,7 +335,8 @@ func FuzzProcessSingleResponse(f *testing.F) { } // The invariant: this must return normally (no panic escaping the // parser's recover, no goroutine leak/deadlock). The returned values - // are intentionally unused beyond confirming completion. - _, _, _ = drainSingleResponse(stream, frag) + // are intentionally unused beyond confirming completion, so collect is + // false to avoid per-token allocations in the hot fuzzing path. + _, _, _ = drainSingleResponse(stream, frag, false) }) } From 153bd965e4b5524c6fa7fd1bc4131f786482d242 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 14 Aug 2026 14:55:43 -0700 Subject: [PATCH 05/38] fix: panic StreamError for bounded TDS allocation guards Convert the issue #420 allocation guards from badStreamPanicf (plain error) to badStreamPanic(fmt.Errorf(...)) so a malformed stream surfaces as a StreamError. Conn.checkBadConn only marks the connection bad for StreamError, so a plain-error panic would leave the poisoned connection in the pool. Also assert the guards panic StreamError in the regression tests. Addresses Copilot review feedback on PR #421. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token.go | 4 ++-- token_alloc_regression_test.go | 29 ++++++++++++++++++----------- types.go | 4 ++-- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/token.go b/token.go index 36384ad66..9cc90b16e 100644 --- a/token.go +++ b/token.go @@ -510,7 +510,7 @@ const _MAX_FEDAUTHINFO_LEN = 1 << 20 func parseFedAuthInfo(r *tdsBuffer) fedAuthInfoStruct { size := r.uint32() if size > _MAX_FEDAUTHINFO_LEN { - badStreamPanicf("federated authentication info size %d exceeds maximum of %d bytes", size, _MAX_FEDAUTHINFO_LEN) + badStreamPanic(fmt.Errorf("federated authentication info size %d exceeds maximum of %d bytes", size, _MAX_FEDAUTHINFO_LEN)) } var STSURL, SPN string @@ -524,7 +524,7 @@ func parseFedAuthInfo(r *tdsBuffer) fedAuthInfoStruct { // within the advertised token size. Reject a count that cannot fit before // allocating, so a bogus count cannot pre-allocate gigabytes of options. if uint64(count)*9+uint64(offset) > uint64(size) { - badStreamPanicf("federated authentication info advertised %d options that do not fit in %d bytes", count, size) + badStreamPanic(fmt.Errorf("federated authentication info advertised %d options that do not fit in %d bytes", count, size)) } opts := make([]fedAuthInfoOpt, count) diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index 537509912..da0338da8 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -21,8 +21,8 @@ func bufFromBytes(stream []byte) *tdsBuffer { } // recoverErr runs fn and returns any panic value coerced to an error. Token -// parsers signal a malformed stream by panicking (badStreamPanic/badStreamPanicf) -// which processSingleResponse recovers into an error token. +// parsers signal a malformed stream by panicking (badStreamPanic) which +// processSingleResponse recovers into an error token. func recoverErr(fn func()) (err error) { defer func() { if r := recover(); r != nil { @@ -37,6 +37,19 @@ func recoverErr(fn func()) (err error) { return nil } +// assertStreamError fails unless err is a StreamError. The allocation guards +// must panic StreamError (not a plain error) so Conn.checkBadConn marks the +// connection bad and drops it from the pool on a malformed stream. +func assertStreamError(t *testing.T, err error) { + t.Helper() + if err == nil { + t.Fatalf("expected a StreamError, got none") + } + if _, ok := err.(StreamError); !ok { + t.Fatalf("expected StreamError, got %T: %v", err, err) + } +} + // TestParseFedAuthInfo_MalformedAllocations is a regression test for issue #420: // parseFedAuthInfo allocated buffers/slices sized directly from attacker // controlled length prefixes before validating them, so a crafted FEDAUTHINFO @@ -78,9 +91,7 @@ func TestParseFedAuthInfo_MalformedAllocations(t *testing.T) { tc := tc t.Run(name, func(t *testing.T) { err := recoverErr(func() { parseFedAuthInfo(bufFromBytes(tc.stream)) }) - if err == nil { - t.Fatalf("expected a stream error, got none") - } + assertStreamError(t, err) assert.Contains(t, err.Error(), tc.wantSub) }) } @@ -109,9 +120,7 @@ func TestReadLongLenType_MalformedLengthPanics(t *testing.T) { err := recoverErr(func() { readLongLenType(&ti, bufFromBytes(build(size)), nil, msdsn.EncodeParameters{}) }) - if err == nil { - t.Fatalf("expected a stream error, got none") - } + assertStreamError(t, err) assert.Contains(t, err.Error(), "maximum LOB size") }) } @@ -127,9 +136,7 @@ func TestReadVariantType_UnderflowPanics(t *testing.T) { err := recoverErr(func() { readVariantTypeWithEncoding(&ti, bufFromBytes(stream), nil, msdsn.EncodeParameters{}) }) - if err == nil { - t.Fatalf("expected a stream error, got none") - } + assertStreamError(t, err) assert.Contains(t, err.Error(), "sql_variant data length") } diff --git a/types.go b/types.go index 9521ebe56..7511d13d2 100644 --- a/types.go +++ b/types.go @@ -577,7 +577,7 @@ func readLongLenType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msd // The advertised size is attacker-controlled; reject anything a real server // cannot produce before using it as an allocation size (OOM DoS, issue #420). if size < 0 || int64(size) > _MAX_PLP_LEN { - badStreamPanicf("TEXT/NTEXT/IMAGE length %d exceeds the maximum LOB size of %d bytes", size, int64(_MAX_PLP_LEN)) + badStreamPanic(fmt.Errorf("TEXT/NTEXT/IMAGE length %d exceeds the maximum LOB size of %d bytes", size, int64(_MAX_PLP_LEN))) } buf := make([]byte, size) r.ReadFull(buf) @@ -665,7 +665,7 @@ func readVariantTypeWithEncoding(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, // so reject an underflowed (negative) or implausibly large value before any // make() to avoid an OOM DoS (issue #420). if datalen := size - 2 - propbytes; datalen < 0 || int64(datalen) > _MAX_PLP_LEN { - badStreamPanicf("sql_variant data length %d is invalid", datalen) + badStreamPanic(fmt.Errorf("sql_variant data length %d is invalid", datalen)) } switch vartype { case typeGuid: From 8ccfd10f94cd889bc8796b4c5b48143cde6b0257 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 14 Aug 2026 15:10:04 -0700 Subject: [PATCH 06/38] fix: bound sql_variant length to 8 KB instead of the LOB ceiling A sql_variant is capped at 8016 bytes on SQL Server and is never a (max)/ LOB type, so bounding its data length at _MAX_PLP_LEN (~2 GiB) still let an attacker-controlled size prefix drive a multi-gigabyte make(). Introduce _MAX_VARIANT_LEN and reject any larger length, and cover the oversize case in the regression test. Addresses Copilot review feedback on PR #421 (issue #420). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token_alloc_regression_test.go | 35 +++++++++++++++++++++++++--------- types.go | 13 +++++++++++-- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index 915587a7e..b0d9ed000 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -128,16 +128,33 @@ func TestReadLongLenType_MalformedLengthPanics(t *testing.T) { // TestReadVariantType_UnderflowPanics is a regression test for issue #420: // readVariantTypeWithEncoding allocated make([]byte, size-2-propbytes) which -// underflowed to a huge value when propbytes exceeded size-2. +// underflowed to a huge value when propbytes exceeded size-2, and also allowed +// an implausibly large size (a sql_variant is capped at ~8 KB on the wire). func TestReadVariantType_UnderflowPanics(t *testing.T) { - // size=3, vartype=typeGuid, propbytes=250 -> 3-2-250 = -249 - stream := []byte{0x03, 0x00, 0x00, 0x00, typeGuid, 0xFA} - ti := typeInfo{} - err := recoverErr(func() { - readVariantTypeWithEncoding(&ti, bufFromBytes(stream), nil, msdsn.EncodeParameters{}) - }) - assertStreamError(t, err) - assert.Contains(t, err.Error(), "sql_variant data length") + build := func(size int32, vartype byte, propbytes byte) []byte { + b := make([]byte, 4) + binary.LittleEndian.PutUint32(b, uint32(size)) + return append(b, vartype, propbytes) + } + + cases := map[string][]byte{ + // size=3, propbytes=250 -> 3-2-250 = -249 (underflow) + "underflow": build(3, typeGuid, 250), + // size just past the sql_variant ceiling -> a bounded but multi-KB+ + // datalen that must still be rejected, not allocated. + "oversize": build(_MAX_VARIANT_LEN+3, typeGuid, 0), + } + for name, stream := range cases { + stream := stream + t.Run(name, func(t *testing.T) { + ti := typeInfo{} + err := recoverErr(func() { + readVariantTypeWithEncoding(&ti, bufFromBytes(stream), nil, msdsn.EncodeParameters{}) + }) + assertStreamError(t, err) + assert.Contains(t, err.Error(), "sql_variant data length") + }) + } } // TestProcessSingleResponse_MalformedNoOOM feeds crafted malformed token streams diff --git a/types.go b/types.go index 7511d13d2..800e02e31 100644 --- a/types.go +++ b/types.go @@ -82,6 +82,14 @@ const _PLP_TERMINATOR = 0x00000000 // malformed stream rather than a value we should try to read. const _MAX_PLP_LEN = 0x7FFFFFFF +// _MAX_VARIANT_LEN is the largest data length a sql_variant value can +// legitimately advertise. A sql_variant is capped at 8016 bytes of storage on +// SQL Server (8000 bytes of data plus metadata) and is never a (max)/LOB type, +// so any larger length is a malformed stream. Bounding it well below +// _MAX_PLP_LEN keeps an attacker-controlled size prefix from driving a +// multi-gigabyte allocation (issue #420). +const _MAX_VARIANT_LEN = 8016 + // TVP COLUMN FLAGS const _TVP_END_TOKEN = 0x00 const _TVP_ROW_TOKEN = 0x01 @@ -663,8 +671,9 @@ func readVariantTypeWithEncoding(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, // size-2-propbytes is the trailing data length and is used below as an // allocation size. It is derived from an attacker-controlled size prefix, // so reject an underflowed (negative) or implausibly large value before any - // make() to avoid an OOM DoS (issue #420). - if datalen := size - 2 - propbytes; datalen < 0 || int64(datalen) > _MAX_PLP_LEN { + // make() to avoid an OOM DoS (issue #420). A sql_variant tops out at ~8 KB + // on the wire, so it is bounded far below the LOB ceiling. + if datalen := size - 2 - propbytes; datalen < 0 || datalen > _MAX_VARIANT_LEN { badStreamPanic(fmt.Errorf("sql_variant data length %d is invalid", datalen)) } switch vartype { From abb94757ce92fa2df5a23c9d922b21f93e2f99d0 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Sun, 16 Aug 2026 15:07:29 -0700 Subject: [PATCH 07/38] test: assert malformed seeds error and valid seeds parse cleanly Address Copilot review suggestions on the TDS fuzz harness: - Add TestProcessSingleResponseMalformedSeeds so a regression that silently accepts an unknown or truncated token is caught, instead of relying only on the fuzz target (which checks for panics) and the boundary test (which excludes malformed seeds). - Assert in TestProcessSingleResponsePacketBoundary that valid seeds never produce an error token, so a seed that fails identically for every fragmentation cannot pass the determinism comparison unnoticed. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 46e48d6ce..1570008fb 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -273,22 +273,51 @@ func fuzzResponseSeeds() [][]byte { return append(validResponseSeeds(), malformedResponseSeeds()...) } +// TestProcessSingleResponseMalformedSeeds asserts that each deliberately broken +// seed is actually rejected: the parser must surface an error token (a +// StreamError produced by its internal recover()) rather than silently +// accepting the unknown or truncated token. Without this, a regression that +// stopped detecting bad streams would go unnoticed, since the fuzz target only +// checks for panics and the boundary test excludes these seeds. +func TestProcessSingleResponseMalformedSeeds(t *testing.T) { + for i, seed := range malformedResponseSeeds() { + seed := seed + t.Run(fmt.Sprintf("seed_%d", i), func(t *testing.T) { + _, sawErr, ok := drainSingleResponse(seed, 0, false) + if !ok { + t.Fatal("failed to frame malformed seed as a single packet") + } + if !sawErr { + t.Fatalf("malformed seed %d did not produce an error token", i) + } + }) + } +} + // TestProcessSingleResponsePacketBoundary asserts that, for known-valid seeds, // the sequence of token types produced by the parser is independent of how the -// stream is fragmented across TDS packets. +// stream is fragmented across TDS packets. It also asserts that no parse fails, +// so a seed that errors identically for every fragmentation cannot pass the +// determinism comparison unnoticed. func TestProcessSingleResponsePacketBoundary(t *testing.T) { for i, seed := range validResponseSeeds() { seed := seed t.Run(fmt.Sprintf("seed_%d", i), func(t *testing.T) { - single, _, ok := drainSingleResponse(seed, 0, true) // one packet + single, singleErr, ok := drainSingleResponse(seed, 0, true) // one packet if !ok { t.Fatal("failed to frame seed as a single packet") } + if singleErr { + t.Fatalf("valid seed %d produced an error token as a single packet", i) + } for _, frag := range []byte{1, 3, 7, 255} { - fragmented, _, ok := drainSingleResponse(seed, frag, true) + fragmented, fragErr, ok := drainSingleResponse(seed, frag, true) if !ok { t.Fatalf("failed to frame seed with frag=%d", frag) } + if fragErr { + t.Fatalf("valid seed %d produced an error token with frag=%d", i, frag) + } if !reflect.DeepEqual(single, fragmented) { t.Fatalf("token sequence differs by packet boundary (frag=%d):\n single=%v\n frag =%v", frag, single, fragmented) From 6f66037c6fe3845d2d6c03e7464534f8427b745b Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Sun, 16 Aug 2026 15:16:07 -0700 Subject: [PATCH 08/38] test: frame TDS fuzz packets by payload size for arbitrary seams Address Copilot review suggestions on the TDS fuzz harness: - frameReplyPackets now takes an explicit per-packet payload size instead of a fixed 1..8 fragment count, so callers can place a packet seam at any byte offset. The fuzz body derives the chunk size from the fuzzed byte, and TestProcessSingleResponsePacketBoundary now enumerates every byte boundary for each valid seed rather than four fixed splits. - Give the ERROR seed's terminating DONE the doneError status bit so it is a protocol-faithful error response per MS-TDS rather than an orphaned error. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 65 +++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 1570008fb..50c280cbc 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -31,21 +31,26 @@ func (t *fuzzTransport) Write(p []byte) (int, error) { return len(p), nil } func (t *fuzzTransport) Close() error { return nil } // frameReplyPackets wraps an arbitrary token stream into one or more packReply -// TDS packets. frag selects a target fragment count in the range 1..8, so the -// same stream is fragmented at different byte boundaries and the parser is -// exercised across packet seams. The target is only a lower bound: any fragment -// larger than fuzzMaxPacketPayload is split further, so a stream longer than -// 8*fuzzMaxPacketPayload can produce more than 8 packets. Every non-final packet -// clears the final status bit; only the last packet sets it. +// TDS packets. chunk is the maximum payload carried by each packet, so it +// directly selects where the packet seams fall: chunk==1 puts a boundary after +// every byte, and a chunk >= len(stream) yields a single packet. A chunk <= 0 +// means "one packet" (subject to the size limit). chunk is clamped to +// fuzzMaxPacketPayload so a packet never exceeds the read buffer. Every +// non-final packet clears the final status bit; only the last packet sets it. +// +// Exposing the payload size (rather than a fixed fragment count) lets callers +// exercise arbitrary seam offsets: the fuzz body derives chunk from a fuzzed +// byte, and the determinism test enumerates every boundary for the valid seeds. // // It returns ok=false when the stream cannot be framed within the uint16 packet // size limit (which, given the input bound in the fuzz body, never happens but // is guarded defensively). -func frameReplyPackets(stream []byte, frag byte) (framed []byte, ok bool) { +func frameReplyPackets(stream []byte, chunk int) (framed []byte, ok bool) { const headerLen = 8 - numFrags := 1 + int(frag)%8 - chunk := (len(stream) + numFrags - 1) / numFrags + if chunk <= 0 || chunk > len(stream) { + chunk = len(stream) + } if chunk < 1 { chunk = 1 } @@ -98,15 +103,16 @@ func newFuzzSession(framed []byte) *tdsSession { // drainSingleResponse runs processSingleResponse against a framed stream and // fully drains the token channel so the reader goroutine never blocks on the -// size-5 buffered channel. When collect is true it returns the ordered list of -// token Go types that were produced (used for the packet-boundary determinism -// invariant); the fuzz target passes collect=false to avoid the per-token -// fmt.Sprintf allocations in the hot path. It also reports whether any -// error/panic token was emitted. processSingleResponse installs its own -// recover(), so a malformed stream surfaces as an error token here rather than -// crashing the harness. -func drainSingleResponse(stream []byte, frag byte, collect bool) (tokenTypes []string, sawError bool, framed bool) { - packets, ok := frameReplyPackets(stream, frag) +// size-5 buffered channel. chunk is the per-packet payload size passed to +// frameReplyPackets (chunk <= 0 means a single packet). When collect is true it +// returns the ordered list of token Go types that were produced (used for the +// packet-boundary determinism invariant); the fuzz target passes collect=false +// to avoid the per-token fmt.Sprintf allocations in the hot path. It also +// reports whether any error/panic token was emitted. processSingleResponse +// installs its own recover(), so a malformed stream surfaces as an error token +// here rather than crashing the harness. +func drainSingleResponse(stream []byte, chunk int, collect bool) (tokenTypes []string, sawError bool, framed bool) { + packets, ok := frameReplyPackets(stream, chunk) if !ok { return nil, false, false } @@ -228,8 +234,9 @@ func validResponseSeeds() [][]byte { concat(colMetadataInt4(), nbcRowNull(), doneToken(tokenDone, doneFinal)), // multiple result sets: DONE(doneMore) then final DONE concat(doneToken(tokenDone, doneMore), doneToken(tokenDone, doneFinal)), - // ERROR -> DONE - concat(infoLikeToken(tokenError), doneToken(tokenDone, doneFinal)), + // ERROR -> DONE(doneError): a statement that completed with an error, + // so the terminating DONE carries the doneError status bit per MS-TDS. + concat(infoLikeToken(tokenError), doneToken(tokenDone, doneError)), // INFO -> DONE concat(infoLikeToken(tokenInfo), doneToken(tokenDone, doneFinal)), // RETURNSTATUS -> DONE @@ -310,17 +317,17 @@ func TestProcessSingleResponsePacketBoundary(t *testing.T) { if singleErr { t.Fatalf("valid seed %d produced an error token as a single packet", i) } - for _, frag := range []byte{1, 3, 7, 255} { - fragmented, fragErr, ok := drainSingleResponse(seed, frag, true) + for chunk := 1; chunk < len(seed); chunk++ { + fragmented, fragErr, ok := drainSingleResponse(seed, chunk, true) if !ok { - t.Fatalf("failed to frame seed with frag=%d", frag) + t.Fatalf("failed to frame seed with chunk=%d", chunk) } if fragErr { - t.Fatalf("valid seed %d produced an error token with frag=%d", i, frag) + t.Fatalf("valid seed %d produced an error token with chunk=%d", i, chunk) } if !reflect.DeepEqual(single, fragmented) { - t.Fatalf("token sequence differs by packet boundary (frag=%d):\n single=%v\n frag =%v", - frag, single, fragmented) + t.Fatalf("token sequence differs by packet boundary (chunk=%d):\n single=%v\n frag =%v", + chunk, single, fragmented) } } }) @@ -362,10 +369,14 @@ func FuzzProcessSingleResponse(f *testing.F) { if len(stream) > 64*1024 { t.Skip() } + // Interpret the fuzzed byte as a packet payload size (1..256) so the + // engine can drive the seam to arbitrary offsets rather than a fixed + // set of even splits. + chunk := 1 + int(frag) // The invariant: this must return normally (no panic escaping the // parser's recover, no goroutine leak/deadlock). The returned values // are intentionally unused beyond confirming completion, so collect is // false to avoid per-token allocations in the hot fuzzing path. - _, _, _ = drainSingleResponse(stream, frag, false) + _, _, _ = drainSingleResponse(stream, chunk, false) }) } From d559b62eac305d386ac7273be89e8683970aaa42 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 12:58:23 -0700 Subject: [PATCH 09/38] fix: clarify TEXT/NTEXT/IMAGE invalid-length error message The guard rejects negative lengths as well as oversize ones, so the error text now describes the value as invalid rather than only 'exceeds' the maximum. Addresses PR review feedback on #420. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types.go b/types.go index 800e02e31..62a288c9d 100644 --- a/types.go +++ b/types.go @@ -585,7 +585,7 @@ func readLongLenType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msd // The advertised size is attacker-controlled; reject anything a real server // cannot produce before using it as an allocation size (OOM DoS, issue #420). if size < 0 || int64(size) > _MAX_PLP_LEN { - badStreamPanic(fmt.Errorf("TEXT/NTEXT/IMAGE length %d exceeds the maximum LOB size of %d bytes", size, int64(_MAX_PLP_LEN))) + badStreamPanic(fmt.Errorf("invalid TEXT/NTEXT/IMAGE length %d: must be between 0 and the maximum LOB size of %d bytes", size, int64(_MAX_PLP_LEN))) } buf := make([]byte, size) r.ReadFull(buf) From a3e1f8348e77bd579212e7cfb754542f696386c9 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 12:58:40 -0700 Subject: [PATCH 10/38] test: compare parsed token values across packet boundaries Strengthen TestProcessSingleResponsePacketBoundary so it normalizes and compares the decoded token contents (row values, DONE status/rowcount, column metadata, return status, order) and the ENVCHANGE session effect (sess.database) instead of only the Go token type. This catches a regression in a cross-packet value path (e.g. the row uint32 read) that would previously pass because both runs still produced []interface{}. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 85 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 19 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 50c280cbc..1614073c9 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -6,6 +6,7 @@ import ( "encoding/binary" "fmt" "reflect" + "strings" "testing" ) @@ -101,20 +102,57 @@ func newFuzzSession(framed []byte) *tdsSession { } } +// normalizeToken renders a token's parsed *contents* (not just its Go type) as +// a stable string, so the packet-boundary invariant can assert that +// fragmentation preserves the decoded values rather than merely the dispatch +// order. Recording only "%T" would let a regression in a cross-packet value +// path (for example the uint32 read behind rowInt4) change 42 to some other +// number while both runs still produced a []interface{}. The representation +// deliberately excludes non-deterministic fields (function pointers inside +// typeInfo, *cryptoMetadata) and keeps only the decoded, comparable values. +func normalizeToken(tok tokenStruct) string { + switch v := tok.(type) { + case doneStruct: + return fmt.Sprintf("done{status=%d curcmd=%d rows=%d errs=%d}", + v.Status, v.CurCmd, v.RowCount, len(v.errors)) + case []columnStruct: + parts := make([]string, len(v)) + for i, c := range v { + parts[i] = fmt.Sprintf("{name=%q type=%d usertype=%d flags=%d}", + c.ColName, c.ti.TypeId, c.UserType, c.Flags) + } + return "cols[" + strings.Join(parts, ",") + "]" + case []interface{}: + return fmt.Sprintf("row%v", v) + case ReturnStatus: + return fmt.Sprintf("returnstatus=%d", int32(v)) + case orderStruct: + return fmt.Sprintf("order%v", v.ColIds) + case error: + // Value irrelevant here: valid seeds never emit an error token, and + // the boundary test already asserts that separately. + return fmt.Sprintf("error:%T", v) + default: + return fmt.Sprintf("%T:%v", tok, tok) + } +} + // drainSingleResponse runs processSingleResponse against a framed stream and // fully drains the token channel so the reader goroutine never blocks on the // size-5 buffered channel. chunk is the per-packet payload size passed to // frameReplyPackets (chunk <= 0 means a single packet). When collect is true it -// returns the ordered list of token Go types that were produced (used for the -// packet-boundary determinism invariant); the fuzz target passes collect=false -// to avoid the per-token fmt.Sprintf allocations in the hot path. It also -// reports whether any error/panic token was emitted. processSingleResponse -// installs its own recover(), so a malformed stream surfaces as an error token -// here rather than crashing the harness. -func drainSingleResponse(stream []byte, chunk int, collect bool) (tokenTypes []string, sawError bool, framed bool) { +// returns the ordered list of normalized token contents that were produced +// (used for the packet-boundary determinism invariant) together with the +// database name recorded on the session, which is the observable side effect of +// an ENVCHANGE token and is not otherwise visible on the channel. The fuzz +// target passes collect=false to avoid the per-token formatting allocations in +// the hot path. It also reports whether any error/panic token was emitted. +// processSingleResponse installs its own recover(), so a malformed stream +// surfaces as an error token here rather than crashing the harness. +func drainSingleResponse(stream []byte, chunk int, collect bool) (tokens []string, dbState string, sawError bool, framed bool) { packets, ok := frameReplyPackets(stream, chunk) if !ok { - return nil, false, false + return nil, "", false, false } sess := newFuzzSession(packets) defer sess.buf.bufClose() @@ -124,13 +162,16 @@ func drainSingleResponse(stream []byte, chunk int, collect bool) (tokenTypes []s for tok := range ch { if collect { - tokenTypes = append(tokenTypes, fmt.Sprintf("%T", tok)) + tokens = append(tokens, normalizeToken(tok)) } if _, isErr := tok.(error); isErr { sawError = true } } - return tokenTypes, sawError, true + // sess.database is mutated by processEnvChg while the goroutine runs; the + // range loop above has returned only after ch is closed, which the parser + // does after it finishes, so this read is safe and final. + return tokens, sess.database, sawError, true } // --- synthetic token-stream builders ------------------------------------- @@ -290,7 +331,7 @@ func TestProcessSingleResponseMalformedSeeds(t *testing.T) { for i, seed := range malformedResponseSeeds() { seed := seed t.Run(fmt.Sprintf("seed_%d", i), func(t *testing.T) { - _, sawErr, ok := drainSingleResponse(seed, 0, false) + _, _, sawErr, ok := drainSingleResponse(seed, 0, false) if !ok { t.Fatal("failed to frame malformed seed as a single packet") } @@ -302,15 +343,17 @@ func TestProcessSingleResponseMalformedSeeds(t *testing.T) { } // TestProcessSingleResponsePacketBoundary asserts that, for known-valid seeds, -// the sequence of token types produced by the parser is independent of how the -// stream is fragmented across TDS packets. It also asserts that no parse fails, -// so a seed that errors identically for every fragmentation cannot pass the -// determinism comparison unnoticed. +// the parser output is independent of how the stream is fragmented across TDS +// packets. It compares the normalized token *contents* (so a cross-packet value +// regression is caught, not just a change in dispatch order) as well as the +// database name recorded on the session (the observable ENVCHANGE side effect). +// It also asserts that no parse fails, so a seed that errors identically for +// every fragmentation cannot pass the determinism comparison unnoticed. func TestProcessSingleResponsePacketBoundary(t *testing.T) { for i, seed := range validResponseSeeds() { seed := seed t.Run(fmt.Sprintf("seed_%d", i), func(t *testing.T) { - single, singleErr, ok := drainSingleResponse(seed, 0, true) // one packet + single, singleDB, singleErr, ok := drainSingleResponse(seed, 0, true) // one packet if !ok { t.Fatal("failed to frame seed as a single packet") } @@ -318,7 +361,7 @@ func TestProcessSingleResponsePacketBoundary(t *testing.T) { t.Fatalf("valid seed %d produced an error token as a single packet", i) } for chunk := 1; chunk < len(seed); chunk++ { - fragmented, fragErr, ok := drainSingleResponse(seed, chunk, true) + fragmented, fragDB, fragErr, ok := drainSingleResponse(seed, chunk, true) if !ok { t.Fatalf("failed to frame seed with chunk=%d", chunk) } @@ -326,9 +369,13 @@ func TestProcessSingleResponsePacketBoundary(t *testing.T) { t.Fatalf("valid seed %d produced an error token with chunk=%d", i, chunk) } if !reflect.DeepEqual(single, fragmented) { - t.Fatalf("token sequence differs by packet boundary (chunk=%d):\n single=%v\n frag =%v", + t.Fatalf("token contents differ by packet boundary (chunk=%d):\n single=%v\n frag =%v", chunk, single, fragmented) } + if singleDB != fragDB { + t.Fatalf("session database differs by packet boundary (chunk=%d): single=%q frag=%q", + chunk, singleDB, fragDB) + } } }) } @@ -377,6 +424,6 @@ func FuzzProcessSingleResponse(f *testing.F) { // parser's recover, no goroutine leak/deadlock). The returned values // are intentionally unused beyond confirming completion, so collect is // false to avoid per-token allocations in the hot fuzzing path. - _, _, _ = drainSingleResponse(stream, chunk, false) + _, _, _, _ = drainSingleResponse(stream, chunk, false) }) } From ff9fd0212b231db0603f03da2ee26da0816306f0 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:03:18 -0700 Subject: [PATCH 11/38] test: compare decoded ERROR contents across packet boundaries The DONE normalization recorded only the count of accumulated ERROR tokens, so a cross-packet regression corrupting an ERROR field (Number, State, Class, Message, ServerName, ProcName, LineNo) while preserving token alignment and count would pass the determinism test. Render the full deterministic contents of each accumulated Error via a new normalizeErrors helper so the ERROR seed verifies decoded values. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 1614073c9..2b53d6863 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -102,6 +102,20 @@ func newFuzzSession(framed []byte) *tdsSession { } } +// normalizeErrors renders the deterministic contents of the ERROR tokens +// accumulated onto a DONE token. Recording only the count would let a +// packet-boundary regression that corrupts a decoded ERROR field (Number, +// State, Class, Message, ServerName, ProcName, LineNo) slip through while the +// token count stayed the same, so the full decoded values are compared here. +func normalizeErrors(errs []Error) string { + parts := make([]string, len(errs)) + for i, e := range errs { + parts[i] = fmt.Sprintf("{num=%d state=%d class=%d msg=%q server=%q proc=%q line=%d}", + e.Number, e.State, e.Class, e.Message, e.ServerName, e.ProcName, e.LineNo) + } + return "[" + strings.Join(parts, ",") + "]" +} + // normalizeToken renders a token's parsed *contents* (not just its Go type) as // a stable string, so the packet-boundary invariant can assert that // fragmentation preserves the decoded values rather than merely the dispatch @@ -113,8 +127,8 @@ func newFuzzSession(framed []byte) *tdsSession { func normalizeToken(tok tokenStruct) string { switch v := tok.(type) { case doneStruct: - return fmt.Sprintf("done{status=%d curcmd=%d rows=%d errs=%d}", - v.Status, v.CurCmd, v.RowCount, len(v.errors)) + return fmt.Sprintf("done{status=%d curcmd=%d rows=%d errs=%s}", + v.Status, v.CurCmd, v.RowCount, normalizeErrors(v.errors)) case []columnStruct: parts := make([]string, len(v)) for i, c := range v { From 18e8c3290fd7f7843f1df9070b27ed95d780b924 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:08:33 -0700 Subject: [PATCH 12/38] fix: fail malformed FEDAUTHINFO/typeid streams as StreamError, guard offset overflow Convert the remaining plain badStreamPanicf sites in parseFedAuthInfo and the readVariantTypeWithEncoding invalid-typeid default to badStreamPanic(fmt.Errorf(...)) so Conn.checkBadConn drops the poisoned connection instead of reusing it. Compute the fed auth opt offset+length bound in uint64 to prevent a uint32 overflow from bypassing the check. Fix a typo in the unreachable-path panic message. Addresses PR review feedback on #420. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token.go | 12 +++++++----- types.go | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/token.go b/token.go index 9cc90b16e..4257206f8 100644 --- a/token.go +++ b/token.go @@ -546,14 +546,16 @@ func parseFedAuthInfo(r *tdsBuffer) fedAuthInfoStruct { for i := uint32(0); i < count; i++ { if opts[i].dataOffset < offset { - badStreamPanicf("Fed auth info opt stated data offset %d is before data begins in packet at %d", - opts[i].dataOffset, offset) + badStreamPanic(fmt.Errorf("fed auth info opt stated data offset %d is before data begins in packet at %d", + opts[i].dataOffset, offset)) // returns via panic } - if opts[i].dataOffset+opts[i].dataLength > size { - badStreamPanicf("Fed auth info opt stated data length %d added to stated offset exceeds size of packet %d", - opts[i].dataOffset+opts[i].dataLength, size) + // Compute in uint64 so an attacker-controlled offset+length cannot + // overflow uint32 and slip past this bounds check (issue #420). + if uint64(opts[i].dataOffset)+uint64(opts[i].dataLength) > uint64(size) { + badStreamPanic(fmt.Errorf("fed auth info opt stated data length %d added to stated offset exceeds size of packet %d", + uint64(opts[i].dataOffset)+uint64(opts[i].dataLength), size)) // returns via panic } diff --git a/types.go b/types.go index 62a288c9d..d54c051ff 100644 --- a/types.go +++ b/types.go @@ -597,9 +597,9 @@ func readLongLenType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msd case typeNText: return decodeNChar(buf) default: - badStreamPanicf("Invalid typeid") + badStreamPanic(fmt.Errorf("invalid type id %d for variable-length type", ti.TypeId)) } - panic("shoulnd't get here") + panic("shouldn't get here") } func writeLongLenType(w io.Writer, ti typeInfo, buf []byte, encoding msdsn.EncodeParameters) (err error) { if buf == nil { From 320dbc2716317c8258e82a4beb26103d22de7c0b Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:08:49 -0700 Subject: [PATCH 13/38] test: correct trailing-bytes seed's documented intent The final-DONE-plus-extra-bytes seed was labeled as trailing-garbage coverage, but processSingleResponse returns on the terminating DONE (doneMore==0) before reading those bytes, so they are never consumed. Relabel the seed to describe the invariant it actually pins -- bytes after the terminating DONE do not alter the parsed token stream or its determinism across packet boundaries -- since arbitrary trailing bytes are exercised by the fuzz target itself. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 2b53d6863..ab049adf4 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -309,9 +309,14 @@ func validResponseSeeds() [][]byte { []byte{byte(tokenOrder), 0x00, 0x00}, doneToken(tokenDone, doneFinal), ), - // valid final DONE followed by trailing garbage: the parser returns on - // the final DONE before reading the garbage, so this still parses cleanly - // and deterministically regardless of packet boundaries. + // final DONE followed by extra bytes within the same logical stream. + // processSingleResponse returns on the terminating DONE (doneMore==0) + // before reading anything after it, so these trailing bytes are never + // consumed. This seed is NOT trailing-garbage coverage (arbitrary + // trailing bytes are exercised by FuzzProcessSingleResponse itself); + // it pins the invariant that bytes following the terminating DONE do + // not alter the parsed token stream or its determinism across packet + // boundaries -- the parser stops cleanly at the final DONE. concat(doneToken(tokenDone, doneFinal), []byte{0xDE, 0xAD, 0xBE, 0xEF}), } } From a2cb70c8a4703a2d7431923b20211517e1ecabf4 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:14:24 -0700 Subject: [PATCH 14/38] test: harden alloc regression helpers per review feedback Guard bufFromBytes against a stream larger than the read buffer so an oversize test input fails with a clear message instead of a slice-bounds panic, and use a bare return in recoverErr to make the deferred panic-to-error conversion obvious. Addresses PR review feedback on #420. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token_alloc_regression_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index 51d558b99..50b43fc69 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -2,6 +2,7 @@ package mssql import ( "encoding/binary" + "fmt" "testing" "github.com/microsoft/go-mssqldb/msdsn" @@ -13,6 +14,9 @@ import ( // exercised directly without a live connection. func bufFromBytes(stream []byte) *tdsBuffer { buf := newTdsBuffer(uint16(1<<15), nil) + if len(stream) > len(buf.rbuf) { + panic(fmt.Sprintf("bufFromBytes: stream of %d bytes exceeds read buffer of %d bytes", len(stream), len(buf.rbuf))) + } copy(buf.rbuf[:len(stream)], stream) buf.rpos = 0 buf.rsize = len(stream) @@ -34,7 +38,8 @@ func recoverErr(fn func()) (err error) { } }() fn() - return nil + // err is set by the deferred recover above; a normal return leaves it nil. + return } // assertStreamError fails unless err is a StreamError. The allocation guards From 199d3fb45a5d92e8c77edf207d3635aaf97f7e31 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:16:37 -0700 Subject: [PATCH 15/38] test: verify INFO/ERROR message values across packet boundaries INFO (and ERROR) decoded values are exposed by the parser only through outs.msgq, never on the token channel, so the boundary test that passed outputs{} compared only the surrounding DONE. A cross-packet regression corrupting a decoded INFO Number/Message/LineNo would pass unnoticed. Wire a sqlexp.ReturnMessage queue into outs.msgq for collection runs, drain it after the parser goroutine finishes (via a private sentinel), and fold the normalized message contents (MsgNotice/MsgError/ MsgRowsAffected/MsgNext/MsgNextResultSet) into the packet-boundary comparison. The fuzz hot path still uses collect=false with no queue. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 73 insertions(+), 6 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index ab049adf4..735f032c4 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -8,6 +8,8 @@ import ( "reflect" "strings" "testing" + + "github.com/golang-sql/sqlexp" ) // fuzzPacketSize is the TDS buffer size used by the response fuzz harness. @@ -102,6 +104,38 @@ func newFuzzSession(framed []byte) *tdsSession { } } +// msgSentinel is a private RawMessage used only to mark the end of the return +// message queue when draining it after the parser goroutine has finished. +type msgSentinel struct{} + +// normalizeMsg renders the deterministic contents of a return-message queue +// entry. INFO/ERROR values and rows-affected counts are exposed by the parser +// only through outs.msgq (never on the token channel), so folding them into the +// packet-boundary comparison is what makes the INFO/ERROR seeds actually verify +// their decoded values rather than merely the surrounding DONE. +func normalizeMsg(m sqlexp.RawMessage) string { + switch v := m.(type) { + case sqlexp.MsgNotice: + if e, ok := v.Message.(Error); ok { + return "notice" + normalizeErrors([]Error{e}) + } + return fmt.Sprintf("notice{%v}", v.Message) + case sqlexp.MsgError: + if e, ok := v.Error.(Error); ok { + return "error" + normalizeErrors([]Error{e}) + } + return fmt.Sprintf("error{%v}", v.Error) + case sqlexp.MsgRowsAffected: + return fmt.Sprintf("rowsaffected=%d", v.Count) + case sqlexp.MsgNext: + return "next" + case sqlexp.MsgNextResultSet: + return "nextresultset" + default: + return fmt.Sprintf("%T", m) + } +} + // normalizeErrors renders the deterministic contents of the ERROR tokens // accumulated onto a DONE token. Recording only the count would let a // packet-boundary regression that corrupts a decoded ERROR field (Number, @@ -158,11 +192,16 @@ func normalizeToken(tok tokenStruct) string { // returns the ordered list of normalized token contents that were produced // (used for the packet-boundary determinism invariant) together with the // database name recorded on the session, which is the observable side effect of -// an ENVCHANGE token and is not otherwise visible on the channel. The fuzz -// target passes collect=false to avoid the per-token formatting allocations in -// the hot path. It also reports whether any error/panic token was emitted. -// processSingleResponse installs its own recover(), so a malformed stream -// surfaces as an error token here rather than crashing the harness. +// an ENVCHANGE token and is not otherwise visible on the channel. Collection +// runs also wire a return-message queue into outs.msgq and append its drained +// contents (INFO/ERROR/rows-affected), because those decoded values are exposed +// only through the queue and never on the token channel; including them makes +// the INFO and ERROR seeds actually verify their parsed values across packet +// boundaries. The fuzz target passes collect=false to avoid the per-token +// formatting allocations and the queue plumbing in the hot path. It also reports +// whether any error/panic token was emitted. processSingleResponse installs its +// own recover(), so a malformed stream surfaces as an error token here rather +// than crashing the harness. func drainSingleResponse(stream []byte, chunk int, collect bool) (tokens []string, dbState string, sawError bool, framed bool) { packets, ok := frameReplyPackets(stream, chunk) if !ok { @@ -172,7 +211,19 @@ func drainSingleResponse(stream []byte, chunk int, collect bool) (tokens []strin defer sess.buf.bufClose() ch := make(chan tokenStruct, 5) - go processSingleResponse(context.Background(), sess, ch, outputs{}) + + var outs outputs + var msgq *sqlexp.ReturnMessage + if collect { + // The queue is buffered (15) and each valid seed enqueues only a + // handful of messages, so it never fills before the parser returns + // and the post-run drain below cannot deadlock. + msgq = &sqlexp.ReturnMessage{} + sqlexp.ReturnMessageInit(msgq) + outs.msgq = msgq + } + + go processSingleResponse(context.Background(), sess, ch, outs) for tok := range ch { if collect { @@ -182,6 +233,22 @@ func drainSingleResponse(stream []byte, chunk int, collect bool) (tokens []strin sawError = true } } + + if collect { + // ch is closed (its close is deferred in processSingleResponse and runs + // on every exit path), so the parser goroutine has returned and every + // message it will enqueue is already buffered. Push a sentinel and drain + // up to it to capture the messages in order without blocking on empty. + _ = sqlexp.ReturnMessageEnqueue(context.Background(), msgq, msgSentinel{}) + for { + m := msgq.Message(context.Background()) + if _, done := m.(msgSentinel); done { + break + } + tokens = append(tokens, "msg:"+normalizeMsg(m)) + } + } + // sess.database is mutated by processEnvChg while the goroutine runs; the // range loop above has returned only after ch is closed, which the parser // does after it finishes, so this read is safe and final. From dfee3015a1f80ad9d534cdc9e7c31e6dde139edd Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:20:01 -0700 Subject: [PATCH 16/38] fix: say 'token' not 'packet' in FEDAUTHINFO stream errors; correct test comment The FEDAUTHINFO bounds checks validate the token size/offsets, not the TDS packet, so the StreamError messages now say 'token'. Reword the readLongLenType regression comment to match what it validates: only the negative-length case is rejected, since a non-negative int32 length is inherently bounded by _MAX_PLP_LEN. Addresses PR review feedback on #420. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token.go | 4 ++-- token_alloc_regression_test.go | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/token.go b/token.go index 4257206f8..a6fc01659 100644 --- a/token.go +++ b/token.go @@ -546,7 +546,7 @@ func parseFedAuthInfo(r *tdsBuffer) fedAuthInfoStruct { for i := uint32(0); i < count; i++ { if opts[i].dataOffset < offset { - badStreamPanic(fmt.Errorf("fed auth info opt stated data offset %d is before data begins in packet at %d", + badStreamPanic(fmt.Errorf("fed auth info opt stated data offset %d is before data begins in token at %d", opts[i].dataOffset, offset)) // returns via panic } @@ -554,7 +554,7 @@ func parseFedAuthInfo(r *tdsBuffer) fedAuthInfoStruct { // Compute in uint64 so an attacker-controlled offset+length cannot // overflow uint32 and slip past this bounds check (issue #420). if uint64(opts[i].dataOffset)+uint64(opts[i].dataLength) > uint64(size) { - badStreamPanic(fmt.Errorf("fed auth info opt stated data length %d added to stated offset exceeds size of packet %d", + badStreamPanic(fmt.Errorf("fed auth info opt stated data length %d added to stated offset exceeds token size %d", uint64(opts[i].dataOffset)+uint64(opts[i].dataLength), size)) // returns via panic } diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index 50b43fc69..d33a5d929 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -103,9 +103,11 @@ func TestParseFedAuthInfo_MalformedAllocations(t *testing.T) { } // TestReadLongLenType_MalformedLengthPanics is a regression test for issue #420: -// readLongLenType used the untrusted int32 length as the buffer size, so a -// negative length aborted with an out-of-range make() and a huge one allocated -// gigabytes. Both must fail the stream cleanly instead. +// readLongLenType used the untrusted int32 length directly as the buffer size, +// so a negative length aborted with an out-of-range make(). The guard now +// rejects a negative length as a clean StreamError. A non-negative length is +// inherently bounded by the protocol LOB maximum (_MAX_PLP_LEN == max int32), +// so this test exercises the negative case that the guard must reject. func TestReadLongLenType_MalformedLengthPanics(t *testing.T) { build := func(size int32) []byte { // textptrsize=1, textptr(1 byte), timestamp(8 bytes), size(int32) From 6ce1f095260bb6123025c51521d3b6878bf10afa Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:24:04 -0700 Subject: [PATCH 17/38] test: drain return-message queue concurrently to avoid harness hang The collection-mode drain waited for processSingleResponse to finish before reading msgq, but ReturnMessageEnqueue blocks once the 15-slot buffer fills. A valid response with enough notices, result sets, or row-count messages would stall the parser before it closed ch and hang this reusable boundary harness. Drain the queue in a goroutine started before the parser, enqueue the sentinel only after ch closes, and join the drainer before reading its output (no data race; verified under -race). Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 54 +++++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 735f032c4..d2d5e7f89 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -193,15 +193,15 @@ func normalizeToken(tok tokenStruct) string { // (used for the packet-boundary determinism invariant) together with the // database name recorded on the session, which is the observable side effect of // an ENVCHANGE token and is not otherwise visible on the channel. Collection -// runs also wire a return-message queue into outs.msgq and append its drained -// contents (INFO/ERROR/rows-affected), because those decoded values are exposed -// only through the queue and never on the token channel; including them makes -// the INFO and ERROR seeds actually verify their parsed values across packet -// boundaries. The fuzz target passes collect=false to avoid the per-token -// formatting allocations and the queue plumbing in the hot path. It also reports -// whether any error/panic token was emitted. processSingleResponse installs its -// own recover(), so a malformed stream surfaces as an error token here rather -// than crashing the harness. +// runs also wire a return-message queue into outs.msgq and drain it (see below) +// to append its contents (INFO/ERROR/rows-affected), because those decoded +// values are exposed only through the queue and never on the token channel; +// including them makes the INFO and ERROR seeds actually verify their parsed +// values across packet boundaries. The fuzz target passes collect=false to +// avoid the per-token formatting allocations and the queue plumbing in the hot +// path. It also reports whether any error/panic token was emitted. +// processSingleResponse installs its own recover(), so a malformed stream +// surfaces as an error token here rather than crashing the harness. func drainSingleResponse(stream []byte, chunk int, collect bool) (tokens []string, dbState string, sawError bool, framed bool) { packets, ok := frameReplyPackets(stream, chunk) if !ok { @@ -214,13 +214,29 @@ func drainSingleResponse(stream []byte, chunk int, collect bool) (tokens []strin var outs outputs var msgq *sqlexp.ReturnMessage + var msgs []string + var msgDone chan struct{} if collect { - // The queue is buffered (15) and each valid seed enqueues only a - // handful of messages, so it never fills before the parser returns - // and the post-run drain below cannot deadlock. msgq = &sqlexp.ReturnMessage{} sqlexp.ReturnMessageInit(msgq) outs.msgq = msgq + // Drain the queue concurrently with the parser: ReturnMessageEnqueue + // blocks once the 15-slot buffer fills, so a response with enough + // notices/result-sets/row-count messages would otherwise stall the + // parser before it closes ch and hang this reusable harness. The + // goroutine writes msgs and is joined (via msgDone) before msgs is + // read below, so there is no data race. + msgDone = make(chan struct{}) + go func() { + defer close(msgDone) + for { + m := msgq.Message(context.Background()) + if _, stop := m.(msgSentinel); stop { + return + } + msgs = append(msgs, "msg:"+normalizeMsg(m)) + } + }() } go processSingleResponse(context.Background(), sess, ch, outs) @@ -236,17 +252,11 @@ func drainSingleResponse(stream []byte, chunk int, collect bool) (tokens []strin if collect { // ch is closed (its close is deferred in processSingleResponse and runs - // on every exit path), so the parser goroutine has returned and every - // message it will enqueue is already buffered. Push a sentinel and drain - // up to it to capture the messages in order without blocking on empty. + // on every exit path), so the parser has enqueued every message it will. + // The sentinel makes the drain goroutine stop once it has consumed them. _ = sqlexp.ReturnMessageEnqueue(context.Background(), msgq, msgSentinel{}) - for { - m := msgq.Message(context.Background()) - if _, done := m.(msgSentinel); done { - break - } - tokens = append(tokens, "msg:"+normalizeMsg(m)) - } + <-msgDone + tokens = append(tokens, msgs...) } // sess.database is mutated by processEnvChg while the goroutine runs; the From ff1bd986e92bc5f052d8d29a036cc12fad2d189d Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:24:07 -0700 Subject: [PATCH 18/38] fix: report FEDAUTHINFO opt offset and length separately in stream error The bounds-check error passed offset+length but labeled it 'data length'; report offset and length as distinct values against the token size for accurate diagnostics. Addresses PR review feedback on #420. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/token.go b/token.go index a6fc01659..6bc05649a 100644 --- a/token.go +++ b/token.go @@ -554,8 +554,8 @@ func parseFedAuthInfo(r *tdsBuffer) fedAuthInfoStruct { // Compute in uint64 so an attacker-controlled offset+length cannot // overflow uint32 and slip past this bounds check (issue #420). if uint64(opts[i].dataOffset)+uint64(opts[i].dataLength) > uint64(size) { - badStreamPanic(fmt.Errorf("fed auth info opt stated data length %d added to stated offset exceeds token size %d", - uint64(opts[i].dataOffset)+uint64(opts[i].dataLength), size)) + badStreamPanic(fmt.Errorf("fed auth info opt data offset %d plus length %d exceeds token size %d", + opts[i].dataOffset, opts[i].dataLength, size)) // returns via panic } From fd5f0a8c50ef810bf3151c4d0b845ee00956c0bd Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:28:59 -0700 Subject: [PATCH 19/38] test: drop trailing-bytes seed from the valid boundary corpus The seed placed extra bytes after a terminating DONE. Under some fragmentations the DONE ends before the packet marked final, so processSingleResponse returns with continuation packets unread; it only looked clean because each run discards its session. That does not belong in validResponseSeeds, which promises well-formed, fully-consumed streams, and it is not truly malformed either (the parser legitimately stops at the final DONE). Arbitrary trailing bytes are already exercised by FuzzProcessSingleResponse, so remove the seed rather than mis-model it. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index d2d5e7f89..5cbe50952 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -386,15 +386,6 @@ func validResponseSeeds() [][]byte { []byte{byte(tokenOrder), 0x00, 0x00}, doneToken(tokenDone, doneFinal), ), - // final DONE followed by extra bytes within the same logical stream. - // processSingleResponse returns on the terminating DONE (doneMore==0) - // before reading anything after it, so these trailing bytes are never - // consumed. This seed is NOT trailing-garbage coverage (arbitrary - // trailing bytes are exercised by FuzzProcessSingleResponse itself); - // it pins the invariant that bytes following the terminating DONE do - // not alter the parsed token stream or its determinism across packet - // boundaries -- the parser stops cleanly at the final DONE. - concat(doneToken(tokenDone, doneFinal), []byte{0xDE, 0xAD, 0xBE, 0xEF}), } } From d7e225371f9d14674262eed6ffe8d8a6b02a39d8 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:33:30 -0700 Subject: [PATCH 20/38] fix: stream TEXT/NTEXT/IMAGE reads to avoid preallocating from length prefix readLongLenType previously called make([]byte, size) sized directly from the attacker-controlled length prefix, allowing a truncated huge length to force an instant multi-GiB allocation. Grow the buffer with io.CopyN as bytes actually arrive so a malformed length fails as a StreamError without preallocation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token_alloc_regression_test.go | 41 ++++++++++++++++++++++------------ types.go | 28 ++++++++++++++++++----- 2 files changed, 49 insertions(+), 20 deletions(-) diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index d33a5d929..130076ba1 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -103,32 +103,45 @@ func TestParseFedAuthInfo_MalformedAllocations(t *testing.T) { } // TestReadLongLenType_MalformedLengthPanics is a regression test for issue #420: -// readLongLenType used the untrusted int32 length directly as the buffer size, -// so a negative length aborted with an out-of-range make(). The guard now -// rejects a negative length as a clean StreamError. A non-negative length is -// inherently bounded by the protocol LOB maximum (_MAX_PLP_LEN == max int32), -// so this test exercises the negative case that the guard must reject. +// readLongLenType used the untrusted int32 length directly as the make() size, +// so a negative length aborted with an out-of-range make() and a huge length +// preallocated gigabytes before any data arrived. The reader now rejects a +// negative length and grows the buffer with the bytes actually received, so a +// truncated huge length fails the stream cleanly instead of preallocating. func TestReadLongLenType_MalformedLengthPanics(t *testing.T) { - build := func(size int32) []byte { - // textptrsize=1, textptr(1 byte), timestamp(8 bytes), size(int32) + build := func(size int32, data []byte) []byte { + // textptrsize=1, textptr(1 byte), timestamp(8 bytes), size(int32), data b := []byte{0x01, 0x00, 0, 0, 0, 0, 0, 0, 0, 0} var s [4]byte binary.LittleEndian.PutUint32(s[:], uint32(size)) - return append(b, s[:]...) + b = append(b, s[:]...) + return append(b, data...) } - sizes := map[string]int32{ - "negative length": -2, + cases := map[string]struct { + stream []byte + wantSub string + }{ + "negative length": { + stream: build(-2, nil), + wantSub: "maximum LOB size", + }, + "truncated huge length": { + // Advertise ~2 GiB but supply only a few bytes: the reader must fail + // the stream instead of preallocating gigabytes (issue #420). + stream: build(0x7FFFFFFF, []byte{0x01, 0x02, 0x03}), + wantSub: "failed", + }, } - for name, size := range sizes { - size := size + for name, tc := range cases { + tc := tc t.Run(name, func(t *testing.T) { ti := typeInfo{TypeId: typeImage} err := recoverErr(func() { - readLongLenType(&ti, bufFromBytes(build(size)), nil, msdsn.EncodeParameters{}) + readLongLenType(&ti, bufFromBytes(tc.stream), nil, msdsn.EncodeParameters{}) }) assertStreamError(t, err) - assert.Contains(t, err.Error(), "maximum LOB size") + assert.Contains(t, err.Error(), tc.wantSub) }) } } diff --git a/types.go b/types.go index d54c051ff..419d4f228 100644 --- a/types.go +++ b/types.go @@ -582,13 +582,29 @@ func readLongLenType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msd if size == -1 { return nil } - // The advertised size is attacker-controlled; reject anything a real server - // cannot produce before using it as an allocation size (OOM DoS, issue #420). - if size < 0 || int64(size) > _MAX_PLP_LEN { - badStreamPanic(fmt.Errorf("invalid TEXT/NTEXT/IMAGE length %d: must be between 0 and the maximum LOB size of %d bytes", size, int64(_MAX_PLP_LEN))) + // The advertised size is attacker-controlled; reject a negative length + // before using it (issue #420). A non-negative int32 is inherently within + // the protocol LOB maximum (_MAX_PLP_LEN == max int32). + if size < 0 { + badStreamPanic(fmt.Errorf("invalid TEXT/NTEXT/IMAGE length %d: must be non-negative and within the maximum LOB size of %d bytes", size, int64(_MAX_PLP_LEN))) } - buf := make([]byte, size) - r.ReadFull(buf) + // Grow the buffer with the bytes actually received rather than + // preallocating the full advertised size, so a hostile server cannot force + // a multi-GiB allocation by advertising a huge length and then truncating + // the stream (OOM DoS, issue #420). A short read fails the stream cleanly + // as an unexpected EOF. + var bb bytes.Buffer + if initialCap := int64(size); initialCap > 0 { + const maxInitialCap = 1 << 16 + if initialCap > maxInitialCap { + initialCap = maxInitialCap + } + bb.Grow(int(initialCap)) + } + if _, err := io.CopyN(&bb, r, int64(size)); err != nil { + badStreamPanic(fmt.Errorf("reading %d-byte TEXT/NTEXT/IMAGE value failed: %w", size, err)) + } + buf := bb.Bytes() switch ti.TypeId { case typeText: return decodeChar(ti.Collation, buf) From 6817aef476d82527922072640b5f873523be891f Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:36:03 -0700 Subject: [PATCH 21/38] test: seed non-zero ERROR/INFO fields and non-empty browse tokens Two boundary-corpus gaps: the ERROR/INFO seed encoded zero/empty Number, Message, ServerName, ProcName and LineNo, so a fragmented-read regression that zeroed any field would still compare equal; and the only TABNAME/COLINFO/ORDER seed had empty bodies, so parseTabName/parseColInfo payload reads and parseOrder's element loop were never crossed by a packet seam. Give infoLikeToken distinct non-zero values for every field (real UCS2 Message/ServerName/ProcName via a new ucs2 helper) and give the browse seed non-empty TABNAME/COLINFO bodies plus an ORDER with one column id. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 63 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 5cbe50952..b15033de4 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -308,19 +308,47 @@ func nbcRowNull() []byte { return []byte{byte(tokenNbcRow), 0x01} } -// infoLikeToken builds an ERROR/INFO token body (they share a layout). The -// Length field is set to the true byte length of the token data that follows -// it, so the seed is spec-faithful even though the current parser ignores it. -func infoLikeToken(tok token) []byte { - body := []byte{ - 0x00, 0x00, 0x00, 0x00, // Number - 0x01, // State - 0x01, // Class - 0x00, 0x00, // Message UsVarChar length = 0 - 0x00, // ServerName BVarChar length = 0 - 0x00, // ProcName BVarChar length = 0 - 0x00, 0x00, 0x00, 0x00, // LineNo +// ucs2 encodes an ASCII string as little-endian UCS-2, the on-the-wire form of +// TDS (B|Us)VarChar payloads used by the ERROR/INFO seeds. +func ucs2(s string) []byte { + b := make([]byte, 0, len(s)*2) + for i := 0; i < len(s); i++ { + b = append(b, s[i], 0x00) } + return b +} + +// infoLikeToken builds an ERROR/INFO token body (they share a layout). Every +// field carries a distinct non-zero value so the packet-boundary test actually +// verifies each decoded field across seams: a fragmented-read regression that +// dropped any one of them to its zero value would change the normalized output. +// The Length field is set to the true byte length of the token data that +// follows it, so the seed is spec-faithful even though the current parser +// ignores it. +func infoLikeToken(tok token) []byte { + var body []byte + num := make([]byte, 4) + binary.LittleEndian.PutUint32(num, 0x11223344) + body = append(body, num...) // Number + body = append(body, 0x07) // State + body = append(body, 0x0E) // Class + // Message: UsVarChar (uint16 char count + UCS2). + msg := "hi" + ml := make([]byte, 2) + binary.LittleEndian.PutUint16(ml, uint16(len(msg))) + body = append(body, ml...) + body = append(body, ucs2(msg)...) + // ServerName: BVarChar (byte char count + UCS2). + body = append(body, 0x01) + body = append(body, ucs2("s")...) + // ProcName: BVarChar (byte char count + UCS2). + body = append(body, 0x01) + body = append(body, ucs2("p")...) + // LineNo. + ln := make([]byte, 4) + binary.LittleEndian.PutUint32(ln, 0x0000007B) + body = append(body, ln...) + out := []byte{byte(tok), 0x00, 0x00} // token id + Length placeholder binary.LittleEndian.PutUint16(out[1:3], uint16(len(body))) return append(out, body...) @@ -379,11 +407,14 @@ func validResponseSeeds() [][]byte { doneToken(tokenDoneInProc, doneFinal), // ENVCHANGE(database) -> DONE concat(envChangeDatabase(), doneToken(tokenDone, doneFinal)), - // TABNAME + COLINFO + ORDER -> DONE + // TABNAME + COLINFO + ORDER -> DONE. Non-empty bodies so the boundary + // test exercises parseTabName/parseColInfo payload reads and + // parseOrder's element loop across packet seams (an ORDER with one + // column id), not just the token dispatch. concat( - []byte{byte(tokenTabName), 0x00, 0x00}, - []byte{byte(tokenColInfo), 0x00, 0x00}, - []byte{byte(tokenOrder), 0x00, 0x00}, + []byte{byte(tokenTabName), 0x04, 0x00, 't', 'a', 'b', 'l'}, // length=4, "tabl" + []byte{byte(tokenColInfo), 0x03, 0x00, 0x01, 0x01, 0x00}, // length=3, ColNum=1, TableNum=1, Status=0 + []byte{byte(tokenOrder), 0x02, 0x00, 0x01, 0x00}, // length=2 bytes, one ColId=1 doneToken(tokenDone, doneFinal), ), } From e39b4716aab8d548bc1a6ebb5456dcb819be3364 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:43:06 -0700 Subject: [PATCH 22/38] test: make TABNAME seed a spec-faithful TDS 7.2 name The browse-token seed's TABNAME body ("tabl") was not a valid TDS 7.2 TABNAME value: its first byte is read as NumParts, so it claimed 116 name parts. parseTabName discards the payload today, so the test still passed, but the corpus entry would be malformed if TABNAME parsing is later strengthened. Add tabNameToken, which encodes NumParts=1 plus one US_VARCHAR part, and use it in the seed so the entry stays valid. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index b15033de4..7a862793d 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -354,6 +354,21 @@ func infoLikeToken(tok token) []byte { return append(out, body...) } +// tabNameToken builds a spec-faithful TDS 7.2 TABNAME token for a single-part +// table name. Per MS-TDS the token value is NumParts (BYTE) followed by that +// many US_VARCHAR name parts (USHORT char count + UCS-2 chars). The Length +// field is the true byte length of the value, so the seed stays valid even if +// parseTabName is later strengthened to actually decode the parts. +func tabNameToken(name string) []byte { + part := make([]byte, 2) + binary.LittleEndian.PutUint16(part, uint16(len(name))) + part = append(part, ucs2(name)...) + body := append([]byte{0x01}, part...) // NumParts = 1 + out := []byte{byte(tokenTabName), 0x00, 0x00} + binary.LittleEndian.PutUint16(out[1:3], uint16(len(body))) + return append(out, body...) +} + // envChangeDatabase builds an ENVCHANGE token announcing a database change. func envChangeDatabase() []byte { // payload: type(1) + new BVarChar("x") + old BVarChar("") @@ -412,9 +427,9 @@ func validResponseSeeds() [][]byte { // parseOrder's element loop across packet seams (an ORDER with one // column id), not just the token dispatch. concat( - []byte{byte(tokenTabName), 0x04, 0x00, 't', 'a', 'b', 'l'}, // length=4, "tabl" - []byte{byte(tokenColInfo), 0x03, 0x00, 0x01, 0x01, 0x00}, // length=3, ColNum=1, TableNum=1, Status=0 - []byte{byte(tokenOrder), 0x02, 0x00, 0x01, 0x00}, // length=2 bytes, one ColId=1 + tabNameToken("t"), // TDS 7.2 TABNAME: NumParts=1, one US_VARCHAR "t" + []byte{byte(tokenColInfo), 0x03, 0x00, 0x01, 0x01, 0x00}, // length=3, ColNum=1, TableNum=1, Status=0 + []byte{byte(tokenOrder), 0x02, 0x00, 0x01, 0x00}, // length=2 bytes, one ColId=1 doneToken(tokenDone, doneFinal), ), } From 70274e4d183fd3c3b04d1218f79ea8f109391edd Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:44:51 -0700 Subject: [PATCH 23/38] test: add non-final DONE + trailing garbage malformed seed The malformed corpus covered an unknown first token and a truncated DONE, but not the trailing-garbage case the PR describes. A final DONE makes processSingleResponse return before reading trailing bytes, so the garbage must follow a non-final DONE (doneMore) to be reached. Add a seed of a valid DONE with doneMore followed by a stray byte, which the parser reads as an unknown token id and recovers into an error token. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 7a862793d..3b44bc498 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -445,6 +445,11 @@ func malformedResponseSeeds() [][]byte { {0x00}, // truncated DONE token (missing bytes) -> recovered error token {byte(tokenDone), 0x00}, + // valid non-final DONE (doneMore) followed by trailing garbage: the + // doneMore status keeps the parser looping past the DONE (a final DONE + // would return first, leaving the garbage unread), so the trailing byte + // is read as an unknown token id and recovered into an error token. + concat(doneToken(tokenDone, doneMore), []byte{0xFF}), } } From 593286217556c6fc765d0915811c1155637d0129 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 13:50:45 -0700 Subject: [PATCH 24/38] test: strengthen valid seeds with non-zero DONE/RETURNSTATUS and real result-set order Three boundary-corpus gaps in the valid seeds: - Every DONE left CurCmd/RowCount zero and omitted doneCount, so the content comparison never crossed those reads or the MsgRowsAffected path. Add doneCountToken and give the ROW seed a DONE with doneCount plus non-zero CurCmd/RowCount. - The sole RETURNSTATUS seed used zero, so a dropped/zeroed four-byte value still normalized identically. Use a non-zero multi-byte value. - The browse-token seed had COLINFO/ORDER referencing column 1 with no preceding COLMETADATA. Prepend the one-column metadata token so it is an actual result-set sequence. Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 3b44bc498..bbd5a371c 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -283,6 +283,19 @@ func doneToken(tok token, status uint16) []byte { return b } +// doneCountToken builds a DONE-family token that reports a row count. The +// doneCount status bit is set and CurCmd/RowCount carry distinct non-zero +// values, so the boundary test exercises those multi-byte cross-packet reads +// and (when collecting messages) the MsgRowsAffected normalization path, which +// a zero-valued DONE would leave untested. +func doneCountToken(tok token, status, curcmd uint16, rowcount uint64) []byte { + b := doneBody(status | doneCount) + b[0] = byte(tok) + binary.LittleEndian.PutUint16(b[3:5], curcmd) + binary.LittleEndian.PutUint64(b[5:13], rowcount) + return b +} + // colMetadataInt4 returns a COLMETADATA token for a single, unnamed int4 column. func colMetadataInt4() []byte { return []byte{ @@ -403,8 +416,10 @@ func validResponseSeeds() [][]byte { return [][]byte{ // empty result set + DONE doneToken(tokenDone, doneFinal), - // COLMETADATA -> ROW -> DONE - concat(colMetadataInt4(), rowInt4(42), doneToken(tokenDone, doneFinal)), + // COLMETADATA -> ROW -> DONE. The terminating DONE sets doneCount and + // carries non-zero CurCmd/RowCount so the content comparison exercises + // those reads and the MsgRowsAffected path, not just status. + concat(colMetadataInt4(), rowInt4(42), doneCountToken(tokenDone, doneFinal, 0x00C1, 7)), // COLMETADATA -> NBCROW(null) -> DONE concat(colMetadataInt4(), nbcRowNull(), doneToken(tokenDone, doneFinal)), // multiple result sets: DONE(doneMore) then final DONE @@ -414,19 +429,23 @@ func validResponseSeeds() [][]byte { concat(infoLikeToken(tokenError), doneToken(tokenDone, doneError)), // INFO -> DONE concat(infoLikeToken(tokenInfo), doneToken(tokenDone, doneFinal)), - // RETURNSTATUS -> DONE - concat(returnStatusToken(0), doneToken(tokenDone, doneFinal)), + // RETURNSTATUS -> DONE. Non-zero multi-byte status so the seed validates + // four-byte decoding across packet seams, not just token dispatch. + concat(returnStatusToken(0x12345678), doneToken(tokenDone, doneFinal)), // DONEPROC (final) doneToken(tokenDoneProc, doneFinal), // DONEINPROC (final) doneToken(tokenDoneInProc, doneFinal), // ENVCHANGE(database) -> DONE concat(envChangeDatabase(), doneToken(tokenDone, doneFinal)), - // TABNAME + COLINFO + ORDER -> DONE. Non-empty bodies so the boundary - // test exercises parseTabName/parseColInfo payload reads and + // COLMETADATA + TABNAME + COLINFO + ORDER -> DONE. The metadata defines + // column 1 that COLINFO/ORDER reference, so this is an actual result-set + // token sequence; the non-empty browse-token bodies also make the + // boundary test exercise parseTabName/parseColInfo payload reads and // parseOrder's element loop across packet seams (an ORDER with one - // column id), not just the token dispatch. + // column id), not just token dispatch. concat( + colMetadataInt4(), tabNameToken("t"), // TDS 7.2 TABNAME: NumParts=1, one US_VARCHAR "t" []byte{byte(tokenColInfo), 0x03, 0x00, 0x01, 0x01, 0x00}, // length=3, ColNum=1, TableNum=1, Status=0 []byte{byte(tokenOrder), 0x02, 0x00, 0x01, 0x00}, // length=2 bytes, one ColId=1 From 79c012b1f490030cd1d4cc419d7a19809b636894 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Fri, 21 Aug 2026 17:50:45 -0700 Subject: [PATCH 25/38] ci: retrigger AppVeyor after account-level build cancellation The required continuous-integration/appveyor/pr check was cancelled by a batch AppVeyor-side event (all PR commits' builds cancelled simultaneously), not by any code change. Empty commit to retrigger CI. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 From 326d4a8c27f48684f3d519c9c76ec2ba516c4f08 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Sat, 22 Aug 2026 09:30:13 -0700 Subject: [PATCH 26/38] test: widen fuzz fragmentation arg to uint16 for full seam coverage FuzzProcessSingleResponse used a byte for the packet-payload size, which capped every packet at 256 bytes. For streams longer than 256 bytes that prevented the target from placing a seam past offset 256 or delivering the stream in the largest legal packet layout. Widen the fuzz argument to uint16 so mutations can drive the seam across the full payload range supported by frameReplyPackets (which still clamps to fuzzMaxPacketPayload). Refs #418 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c3e268ca-c6f4-4edb-826c-3a4018cf939c --- token_fuzz_test.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index bbd5a371c..144e807f0 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -559,23 +559,24 @@ func TestProcessSingleResponsePacketBoundary(t *testing.T) { // test-infrastructure layer. func FuzzProcessSingleResponse(f *testing.F) { for _, seed := range fuzzResponseSeeds() { - f.Add(seed, byte(0)) - f.Add(seed, byte(3)) + f.Add(seed, uint16(0)) + f.Add(seed, uint16(3)) } // A couple of raw single-token seeds for extra coverage. - f.Add([]byte{byte(tokenColMetadata)}, byte(0)) - f.Add([]byte{}, byte(0)) + f.Add([]byte{byte(tokenColMetadata)}, uint16(0)) + f.Add([]byte{}, uint16(0)) - f.Fuzz(func(t *testing.T, stream []byte, frag byte) { + f.Fuzz(func(t *testing.T, stream []byte, frag uint16) { // Bound input size to keep framing and allocations reasonable. A TDS // packet length is a uint16, and the read buffer is 32 KiB, so very // large inputs would either fail to frame or blow the buffer. if len(stream) > 64*1024 { t.Skip() } - // Interpret the fuzzed byte as a packet payload size (1..256) so the - // engine can drive the seam to arbitrary offsets rather than a fixed - // set of even splits. + // Interpret the fuzzed value as a packet payload size (1..65536) so the + // engine can drive the seam to any offset across the full payload range + // that frameReplyPackets supports, not just the first 256 bytes. + // frameReplyPackets clamps chunk to fuzzMaxPacketPayload. chunk := 1 + int(frag) // The invariant: this must return normally (no panic escaping the // parser's recover, no goroutine leak/deadlock). The returned values From d2b7237d36d18b9e83a244224236feaaaf06c801 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Sat, 22 Aug 2026 09:41:41 -0700 Subject: [PATCH 27/38] fix: bound COLMETADATA column count to prevent OOM from malformed streams parseColMetadata72 allocated make([]columnStruct, count) directly from the attacker-controlled uint16 column count. Even within the uint16 ceiling a count of 0xFFFE preallocates many MiB of columnStruct backing array, and repeated malformed responses drive memory pressure. Cap the count at a protocol-sane maximum and fail the stream via badStreamPanic before allocating. Adds a focused regression test. Fixes #420 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token.go | 12 ++++++++++++ token_alloc_regression_test.go | 16 ++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/token.go b/token.go index 6bc05649a..b31f16bfe 100644 --- a/token.go +++ b/token.go @@ -664,6 +664,15 @@ func parseFeatureExtAck(r *tdsBuffer) featureExtAck { return ack } +// _MAX_COLUMN_COUNT bounds the number of columns a COLMETADATA token may +// advertise before we allocate the backing slice. SQL Server limits a result +// set to 4096 columns per SELECT statement, so this generous cap leaves ample +// headroom for hidden/browse-mode columns while keeping an attacker-controlled +// uint16 count (up to 0xFFFE) from preallocating many MiB of columnStruct +// backing array on every malformed response (OOM DoS, issue #420). A violation +// fails the stream as a StreamError. +const _MAX_COLUMN_COUNT = 0x4000 + // http://msdn.microsoft.com/en-us/library/dd357363.aspx func parseColMetadata72(r *tdsBuffer, s *tdsSession) (columns []columnStruct) { count := r.uint16() @@ -671,6 +680,9 @@ func parseColMetadata72(r *tdsBuffer, s *tdsSession) (columns []columnStruct) { // no metadata is sent return nil } + if count > _MAX_COLUMN_COUNT { + badStreamPanic(fmt.Errorf("column count %d exceeds maximum of %d columns", count, _MAX_COLUMN_COUNT)) + } columns = make([]columnStruct, count) var cekTable *cekTable if s.alwaysEncrypted { diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index 130076ba1..49cc6d4ec 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -177,6 +177,22 @@ func TestReadVariantType_UnderflowPanics(t *testing.T) { } } +// TestParseColMetadata72_BogusCountPanics is a regression test for issue #420: +// parseColMetadata72 allocated make([]columnStruct, count) directly from the +// attacker-controlled uint16 column count. Even within the uint16 ceiling a +// count of 0xFFFE preallocates many MiB of columnStruct backing array, so a +// bogus count must now fail the stream as a StreamError before allocating. +func TestParseColMetadata72_BogusCountPanics(t *testing.T) { + // count just past the cap (little-endian uint16), no column data. + var b [2]byte + binary.LittleEndian.PutUint16(b[:], uint16(_MAX_COLUMN_COUNT+1)) + err := recoverErr(func() { + parseColMetadata72(bufFromBytes(b[:]), &tdsSession{}) + }) + assertStreamError(t, err) + assert.Contains(t, err.Error(), "exceeds maximum") +} + // TestProcessSingleResponse_MalformedNoOOM feeds crafted malformed token streams // (framed as reply packets) through the full response parser and asserts each is // turned into an error token rather than hanging or exhausting memory. These are From a8d58bedb54e2fb402089fea36703f9f580dc0b5 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Sat, 22 Aug 2026 23:27:20 -0700 Subject: [PATCH 28/38] docs: explain why the COLMETADATA and FEDAUTHINFO allocation caps are safe Record the reasoning behind the two constant caps so a future reader does not have to re-derive it. _MAX_COLUMN_COUNT: note that COLMETADATA describes a result set, so the binding limit is 4096 columns per SELECT, not the per-table limits (1024, or 30000 with a sparse column set) a reader might otherwise reach for. _MAX_FEDAUTHINFO_LEN: spell out that the token carries only a short STSURL and SPN, so 1 MiB is many times any legitimate token. Comment-only change; no behavior change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token.go | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/token.go b/token.go index b31f16bfe..c62af5015 100644 --- a/token.go +++ b/token.go @@ -500,11 +500,16 @@ type fedAuthInfoOpt struct { dataLength, dataOffset uint32 } -// _MAX_FEDAUTHINFO_LEN bounds the total FEDAUTHINFO token size. The token only -// carries a small STSURL and SPN, so any larger advertised size is a malformed -// or hostile stream rather than something we should allocate for. The cap keeps -// an attacker-controlled length prefix from driving an unbounded allocation -// (OOM DoS, issue #420); a violation fails the stream as a StreamError. +// _MAX_FEDAUTHINFO_LEN bounds the total FEDAUTHINFO token size. The token +// carries only a STSURL and SPN: the STSURL is a login endpoint and the SPN a +// service principal name, both short URL/UPN-shaped strings encoded in UTF-16, +// so a few hundred bytes each is realistic and even a pathological pair stays +// well under 64 KiB. 1 MiB is therefore many times any legitimate token while +// still small enough that rejecting past it costs nothing; any larger advertised +// size is a malformed or hostile stream rather than something we should allocate +// for. The cap keeps an attacker-controlled length prefix from driving an +// unbounded allocation (OOM DoS, issue #420); a violation fails the stream as a +// StreamError. const _MAX_FEDAUTHINFO_LEN = 1 << 20 func parseFedAuthInfo(r *tdsBuffer) fedAuthInfoStruct { @@ -665,12 +670,15 @@ func parseFeatureExtAck(r *tdsBuffer) featureExtAck { } // _MAX_COLUMN_COUNT bounds the number of columns a COLMETADATA token may -// advertise before we allocate the backing slice. SQL Server limits a result -// set to 4096 columns per SELECT statement, so this generous cap leaves ample -// headroom for hidden/browse-mode columns while keeping an attacker-controlled -// uint16 count (up to 0xFFFE) from preallocating many MiB of columnStruct -// backing array on every malformed response (OOM DoS, issue #420). A violation -// fails the stream as a StreamError. +// advertise before we allocate the backing slice. COLMETADATA describes a +// result set, not a table, so the binding limit is SQL Server's maximum of +// 4096 columns *per SELECT statement* — not the per-table limits a reader might +// otherwise reach for (1024 regular columns, or 30000 with a sparse column +// set). This 0x4000 (16384) cap is a comfortable 4x over that binding limit, +// leaving ample headroom for hidden/browse-mode columns while keeping an +// attacker-controlled uint16 count (up to 0xFFFE) from preallocating many MiB +// of columnStruct backing array on every malformed response (OOM DoS, +// issue #420). A violation fails the stream as a StreamError. const _MAX_COLUMN_COUNT = 0x4000 // http://msdn.microsoft.com/en-us/library/dd357363.aspx From 3b04116b8c8790e9e3c81c643c4eacd836f82dd5 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Sun, 23 Aug 2026 15:46:28 -0700 Subject: [PATCH 29/38] refactor: grow COLMETADATA columns incrementally instead of capping count Replace the _MAX_COLUMN_COUNT cap in parseColMetadata72 with incremental slice growth. Rather than rejecting a large column count, the reader now appends one columnStruct per column as it is actually parsed from the wire, so the backing array only grows as far as the server genuinely sent data. A bogus count that outruns the stream fails via badStreamPanic (EOF) after only the present columns are read, keeping the client decoupled from the declared count while still preventing the OOM DoS from issue #420. Fixes #420 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token.go | 38 ++++++++++++++++++---------------- token_alloc_regression_test.go | 17 ++++++++------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/token.go b/token.go index c62af5015..e1fcd2363 100644 --- a/token.go +++ b/token.go @@ -669,18 +669,6 @@ func parseFeatureExtAck(r *tdsBuffer) featureExtAck { return ack } -// _MAX_COLUMN_COUNT bounds the number of columns a COLMETADATA token may -// advertise before we allocate the backing slice. COLMETADATA describes a -// result set, not a table, so the binding limit is SQL Server's maximum of -// 4096 columns *per SELECT statement* — not the per-table limits a reader might -// otherwise reach for (1024 regular columns, or 30000 with a sparse column -// set). This 0x4000 (16384) cap is a comfortable 4x over that binding limit, -// leaving ample headroom for hidden/browse-mode columns while keeping an -// attacker-controlled uint16 count (up to 0xFFFE) from preallocating many MiB -// of columnStruct backing array on every malformed response (OOM DoS, -// issue #420). A violation fails the stream as a StreamError. -const _MAX_COLUMN_COUNT = 0x4000 - // http://msdn.microsoft.com/en-us/library/dd357363.aspx func parseColMetadata72(r *tdsBuffer, s *tdsSession) (columns []columnStruct) { count := r.uint16() @@ -688,18 +676,31 @@ func parseColMetadata72(r *tdsBuffer, s *tdsSession) (columns []columnStruct) { // no metadata is sent return nil } - if count > _MAX_COLUMN_COUNT { - badStreamPanic(fmt.Errorf("column count %d exceeds maximum of %d columns", count, _MAX_COLUMN_COUNT)) - } - columns = make([]columnStruct, count) var cekTable *cekTable if s.alwaysEncrypted { // column encryption key list cekTable = readCekTable(r) } - for i := range columns { - column := &columns[i] + // Grow the column slice as each column is actually parsed rather than + // preallocating make([]columnStruct, count). count is an attacker-controlled + // uint16 and columnStruct is large, so pre-sizing from the count alone lets a + // bogus value (up to 0xFFFE) commit many MiB up front before any of the + // backing bytes are read (OOM DoS, issue #420). Parsing each column consumes + // bytes from the stream, so a count that outruns the data fails via + // badStreamPanic (EOF) after only the columns actually present are read; the + // slice therefore never grows past what the server genuinely sent, which also + // decouples the client from the declared count. The capacity hint is bounded + // so the count cannot drive even the first allocation. + const initialColumnCap = 64 + capHint := int(count) + if capHint > initialColumnCap { + capHint = initialColumnCap + } + columns = make([]columnStruct, 0, capHint) + + for i := 0; i < int(count); i++ { + var column columnStruct baseTi := getBaseTypeInfo(r, true) typeInfo := readTypeInfo(r, baseTi.TypeId, column.cryptoMeta, s.encoding) typeInfo.UserType = baseTi.UserType @@ -720,6 +721,7 @@ func parseColMetadata72(r *tdsBuffer, s *tdsSession) (columns []columnStruct) { } column.ColName = r.BVarChar() + columns = append(columns, column) } return columns } diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index 49cc6d4ec..e1ad2fc43 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -180,17 +180,20 @@ func TestReadVariantType_UnderflowPanics(t *testing.T) { // TestParseColMetadata72_BogusCountPanics is a regression test for issue #420: // parseColMetadata72 allocated make([]columnStruct, count) directly from the // attacker-controlled uint16 column count. Even within the uint16 ceiling a -// count of 0xFFFE preallocates many MiB of columnStruct backing array, so a -// bogus count must now fail the stream as a StreamError before allocating. +// count of 0xFFFE preallocates many MiB of columnStruct backing array. The +// reader now grows the slice as each column is actually parsed, so a bogus +// count with no backing data fails the stream as a StreamError (EOF while +// reading the first column) instead of preallocating. func TestParseColMetadata72_BogusCountPanics(t *testing.T) { - // count just past the cap (little-endian uint16), no column data. - var b [2]byte - binary.LittleEndian.PutUint16(b[:], uint16(_MAX_COLUMN_COUNT+1)) + // A bogus-huge column count (0xFFFE, not the 0xFFFF "no metadata" sentinel) + // with no column data behind it. Parsing the first column must run past the + // end of the stream and panic a StreamError rather than allocating a giant + // slice up front. + stream := []byte{0xFE, 0xFF} err := recoverErr(func() { - parseColMetadata72(bufFromBytes(b[:]), &tdsSession{}) + parseColMetadata72(bufFromBytes(stream), &tdsSession{}) }) assertStreamError(t, err) - assert.Contains(t, err.Error(), "exceeds maximum") } // TestProcessSingleResponse_MalformedNoOOM feeds crafted malformed token streams From 82a076c1d24d5f274d32871d2e81c025afa317cc Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Thu, 27 Aug 2026 23:18:30 -0700 Subject: [PATCH 30/38] docs: clarify COLMETADATA growth and sql_variant bound comments Address reviewer feedback on two explanatory comments: - parseColMetadata72: drop the misleading 'decouples the client from the declared count' phrasing. The loop still iterates count times per protocol; the actual guarantee is that no allocation is pre-sized from count, so the slice never grows past the columns the server actually sent. - _MAX_VARIANT_LEN: reword the size math so 8016 is described consistently as the sql_variant total cap (8000 bytes of data plus up to 16 bytes of type metadata), used as a conservative upper bound for the trailing allocation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token.go | 13 +++++++------ types.go | 13 ++++++++----- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/token.go b/token.go index e1fcd2363..d8379d09f 100644 --- a/token.go +++ b/token.go @@ -686,12 +686,13 @@ func parseColMetadata72(r *tdsBuffer, s *tdsSession) (columns []columnStruct) { // preallocating make([]columnStruct, count). count is an attacker-controlled // uint16 and columnStruct is large, so pre-sizing from the count alone lets a // bogus value (up to 0xFFFE) commit many MiB up front before any of the - // backing bytes are read (OOM DoS, issue #420). Parsing each column consumes - // bytes from the stream, so a count that outruns the data fails via - // badStreamPanic (EOF) after only the columns actually present are read; the - // slice therefore never grows past what the server genuinely sent, which also - // decouples the client from the declared count. The capacity hint is bounded - // so the count cannot drive even the first allocation. + // backing bytes are read (OOM DoS, issue #420). The loop still iterates count + // times per protocol, but parsing each column consumes bytes from the stream, + // so a count that outruns the data fails via badStreamPanic (EOF) after only + // the columns actually present are read; the slice therefore never grows past + // what the server genuinely sent, no matter how large the declared count is. + // The capacity hint is bounded so the count cannot drive even the first + // allocation. const initialColumnCap = 64 capHint := int(count) if capHint > initialColumnCap { diff --git a/types.go b/types.go index 419d4f228..3b2b1894b 100644 --- a/types.go +++ b/types.go @@ -83,11 +83,14 @@ const _PLP_TERMINATOR = 0x00000000 const _MAX_PLP_LEN = 0x7FFFFFFF // _MAX_VARIANT_LEN is the largest data length a sql_variant value can -// legitimately advertise. A sql_variant is capped at 8016 bytes of storage on -// SQL Server (8000 bytes of data plus metadata) and is never a (max)/LOB type, -// so any larger length is a malformed stream. Bounding it well below -// _MAX_PLP_LEN keeps an attacker-controlled size prefix from driving a -// multi-gigabyte allocation (issue #420). +// legitimately advertise. On SQL Server a sql_variant occupies at most 8016 +// bytes total, which is 8000 bytes of value data plus up to 16 bytes of type +// metadata. We use that 8016 total as a single conservative upper bound for the +// trailing data allocation (rather than tracking the exact per-type metadata +// size), and a sql_variant is never a (max)/LOB type, so any larger length is a +// malformed stream. Bounding it well below _MAX_PLP_LEN keeps an +// attacker-controlled size prefix from driving a multi-gigabyte allocation +// (issue #420). const _MAX_VARIANT_LEN = 8016 // TVP COLUMN FLAGS From 977eae3fa6d980b2b2dc11e380263a83aee8197c Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Tue, 1 Sep 2026 22:51:37 -0700 Subject: [PATCH 31/38] test: measure allocation in COLMETADATA bogus-count regression test The prior TestParseColMetadata72_BogusCountPanics only asserted a StreamError, but the pre-fix make([]columnStruct, count) code also panicked the same StreamError on input {0xFE,0xFF} (it allocated ~14 MiB, entered the loop, and the first getBaseTypeInfo read hit EOF). Asserting the error alone therefore passed against both implementations and did not guard the regression. Measure the property that actually changed: wrap the parse in runtime.ReadMemStats and assert TotalAlloc grows by less than 1 MiB. make([]columnStruct, 0xFFFE) is ~14 MiB (verified: the test fails reporting 15343872 bytes when the fix is reverted), while the incremental path allocates only a 64-element capacity hint, so the 1 MiB ceiling separates the two with headroom. A future revert to make([]columnStruct, count) now turns this test red. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token_alloc_regression_test.go | 40 +++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index e1ad2fc43..20100e386 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -3,6 +3,7 @@ package mssql import ( "encoding/binary" "fmt" + "runtime" "testing" "github.com/microsoft/go-mssqldb/msdsn" @@ -179,21 +180,44 @@ func TestReadVariantType_UnderflowPanics(t *testing.T) { // TestParseColMetadata72_BogusCountPanics is a regression test for issue #420: // parseColMetadata72 allocated make([]columnStruct, count) directly from the -// attacker-controlled uint16 column count. Even within the uint16 ceiling a -// count of 0xFFFE preallocates many MiB of columnStruct backing array. The -// reader now grows the slice as each column is actually parsed, so a bogus -// count with no backing data fails the stream as a StreamError (EOF while -// reading the first column) instead of preallocating. +// attacker-controlled uint16 column count, committing many MiB of columnStruct +// backing array for a bogus count (up to 0xFFFE) before reading any column +// bytes. The reader now grows the slice as each column is actually parsed. +// +// Asserting only that the parse fails as a StreamError does NOT guard this +// regression: the pre-fix code panicked the same StreamError on this exact input +// (it ran make([]columnStruct, 0xFFFE), entered the loop, and the first +// getBaseTypeInfo read hit EOF and badStreamPanic'd) — the only difference was +// the ~13 MiB allocated first, which is the whole point of the fix. The property +// that actually changed is the allocation, so the test measures it. func TestParseColMetadata72_BogusCountPanics(t *testing.T) { // A bogus-huge column count (0xFFFE, not the 0xFFFF "no metadata" sentinel) - // with no column data behind it. Parsing the first column must run past the - // end of the stream and panic a StreamError rather than allocating a giant - // slice up front. + // with no column data behind it. stream := []byte{0xFE, 0xFF} + + var before, after runtime.MemStats + runtime.ReadMemStats(&before) err := recoverErr(func() { parseColMetadata72(bufFromBytes(stream), &tdsSession{}) }) + runtime.ReadMemStats(&after) + + // The parse must still fail cleanly as a StreamError once the wire data runs + // out (EOF while reading the first column). assertStreamError(t, err) + + // And it must not pre-size the column slice from the declared count. + // make([]columnStruct, 0xFFFE) is well over 10 MiB; the incremental path + // allocates only a bounded capacity hint (64 elements, a few KiB). TotalAlloc + // is cumulative process-wide bytes, so it only ever grows and GC cannot mask + // the pre-fix allocation. A 1 MiB ceiling sits an order of magnitude below + // that allocation and far above the new one, separating the two + // implementations with headroom on both sides. If someone later "simplifies" + // this back to make([]columnStruct, count), this bound goes red even though + // the StreamError assertion above would still pass. + if grew := after.TotalAlloc - before.TotalAlloc; grew > 1<<20 { + t.Fatalf("parsing a bogus column count allocated %d bytes; parseColMetadata72 must not pre-size the column slice from the declared count", grew) + } } // TestProcessSingleResponse_MalformedNoOOM feeds crafted malformed token streams From 5ad6a3f0f8c473efe4b0ff9f2275a814ea39e29b Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Tue, 1 Sep 2026 23:00:53 -0700 Subject: [PATCH 32/38] test: loosen COLMETADATA alloc ceiling to avoid parallel-test flakiness runtime.MemStats.TotalAlloc is process-global and this package runs many t.Parallel() tests, so a 1 MiB ceiling on the measurement window risked flaking on incidental background allocations. Raise it to 4 MiB: still an order of magnitude below the >14 MiB the pre-fix make([]columnStruct, 0xFFFE) commits and well above the bounded path's few-KiB usage, so it keeps the regression guard while tolerating background noise. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token_alloc_regression_test.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index 20100e386..eb01cdfdc 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -207,15 +207,23 @@ func TestParseColMetadata72_BogusCountPanics(t *testing.T) { assertStreamError(t, err) // And it must not pre-size the column slice from the declared count. - // make([]columnStruct, 0xFFFE) is well over 10 MiB; the incremental path + // make([]columnStruct, 0xFFFE) is well over 14 MiB; the incremental path // allocates only a bounded capacity hint (64 elements, a few KiB). TotalAlloc // is cumulative process-wide bytes, so it only ever grows and GC cannot mask - // the pre-fix allocation. A 1 MiB ceiling sits an order of magnitude below - // that allocation and far above the new one, separating the two - // implementations with headroom on both sides. If someone later "simplifies" - // this back to make([]columnStruct, count), this bound goes red even though - // the StreamError assertion above would still pass. - if grew := after.TotalAlloc - before.TotalAlloc; grew > 1<<20 { + // the pre-fix allocation. + // + // The ceiling is deliberately loose. TotalAlloc is process-global, and other + // tests in this package call t.Parallel(); although a non-parallel test like + // this one does not overlap the parallel batch, background goroutines left by + // earlier tests can still allocate during the measurement window. A very tight + // bound would make this test flaky. 4 MiB sits far above any such incidental + // noise (the bounded path plus background stays well under 1 MiB in practice) + // yet still an order of magnitude below the >14 MiB the pre-fix make() commits, + // so it separates the two implementations with headroom on both sides. If + // someone later "simplifies" this back to make([]columnStruct, count), this + // bound goes red even though the StreamError assertion above would still pass. + const allocCeiling = 4 << 20 + if grew := after.TotalAlloc - before.TotalAlloc; grew > allocCeiling { t.Fatalf("parsing a bogus column count allocated %d bytes; parseColMetadata72 must not pre-size the column slice from the declared count", grew) } } From 8640697fa0aa8ef064fba36ee47403c10d207bcb Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Thu, 3 Sep 2026 01:48:36 -0700 Subject: [PATCH 33/38] test: cover FEDAUTHINFO dataOffset+dataLength uint32 overflow guard The uint64 overflow check in parseFedAuthInfo's second loop had no regression test: all existing TestParseFedAuthInfo_MalformedAllocations cases panic at the size cap or the option-count check above it, so nothing in the suite reached the dataOffset13 is false so it passed, and data[0:0xFFFFFFF7] then panicked a slice-bounds runtime.Error. processSingleResponse recovers that as a generic panic rather than a StreamError, so checkBadConn would not mark the connection bad and the poisoned connection returns to the pool. In uint64 the sum is ~4 GiB, so it fails cleanly as a StreamError. Verified the test-of-the-test: reverting the check to uint32 makes this case fail with 'slice bounds out of range [:4294967287] with capacity 0', exactly the mis-recovered panic the fix prevents. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token_alloc_regression_test.go | 25 +++++++++++++++++++++++++ token_fuzz_test.go | 11 +++++++++++ 2 files changed, 36 insertions(+) diff --git a/token_alloc_regression_test.go b/token_alloc_regression_test.go index eb01cdfdc..ee130492c 100644 --- a/token_alloc_regression_test.go +++ b/token_alloc_regression_test.go @@ -91,6 +91,31 @@ func TestParseFedAuthInfo_MalformedAllocations(t *testing.T) { stream: append(u32(_MAX_FEDAUTHINFO_LEN+1), u32(0)...), wantSub: "exceeds maximum", }, + // An option whose dataOffset+dataLength overflows uint32. This reaches the + // second loop (all cases above panic at the size cap or the option-count + // check, so nothing else exercises it) and is the only input that + // distinguishes the uint64 overflow check from the pre-fix uint32 one. + // Body: size=13, count=1, then one option {ID=STSURL, dataLength=0xFFFFFFF7, + // dataOffset=13}. After the header loop offset==13, so data is empty. Pre-fix + // the check computed dataOffset+dataLength in uint32: 13+0xFFFFFFF7 wraps to + // 4, 4 > 13 is false, the check passes, and data[0:0xFFFFFFF7] then panics a + // slice-bounds runtime.Error — which processSingleResponse recovers as a + // generic panic, not a StreamError, so the connection is not marked bad and + // is returned to the pool poisoned. In uint64 the sum is ~4 GiB, so it fails + // cleanly as a StreamError. This case fails without the uint64 change. + "data offset plus length overflow": func() struct { + stream []byte + wantSub string + } { + b := append(u32(13), u32(1)...) + b = append(b, fedAuthInfoSTSURL) + b = append(b, u32(0xFFFFFFF7)...) // dataLength (read before dataOffset) + b = append(b, u32(13)...) // dataOffset == offset, so it clears the "before data begins" check + return struct { + stream []byte + wantSub string + }{stream: b, wantSub: "exceeds token size"} + }(), } for name, tc := range cases { diff --git a/token_fuzz_test.go b/token_fuzz_test.go index 7d2a85d7f..d20e4f962 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -582,6 +582,17 @@ func FuzzProcessSingleResponse(f *testing.F) { f.Add([]byte{byte(tokenFedAuthInfo), 8, 0, 0, 0, 0xFF, 0xFF, 0xFF, 0xFF}, uint16(0)) // COLMETADATA with a bogus-huge column count (0xFFFE) and no column data. f.Add([]byte{byte(tokenColMetadata), 0xFE, 0xFF}, uint16(0)) + // FEDAUTHINFO option whose dataOffset+dataLength overflows uint32: size=13, + // count=1, then {ID=STSURL, dataLength=0xFFFFFFF7, dataOffset=13}. Pre-fix the + // uint32 sum wrapped past the bounds check and drove a slice-bounds panic; it + // must now surface as an error token. + f.Add([]byte{byte(tokenFedAuthInfo), + 13, 0, 0, 0, // size + 1, 0, 0, 0, // count + fedAuthInfoSTSURL, + 0xF7, 0xFF, 0xFF, 0xFF, // dataLength + 13, 0, 0, 0, // dataOffset + }, uint16(0)) f.Fuzz(func(t *testing.T, stream []byte, frag uint16) { // Bound input size to keep framing and allocations reasonable. A TDS From d0f5dec203b5a8aeb72f55df948765d6d3fbf7d1 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Thu, 10 Sep 2026 17:23:03 -0700 Subject: [PATCH 34/38] fix: bound CEK table allocations and validate key value counts Grow CEK tables only after parsing entries and enforce SQL Server's documented two-value key-rotation limit. Read complete metadata versions and widen UTF-16 lengths before multiplication. Add allocation, truncation, rotation, packet-boundary, and encrypted-response fuzz regressions. Fixes #420 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token.go | 28 ++-- token_cek_regression_test.go | 267 +++++++++++++++++++++++++++++++++++ token_fuzz_test.go | 15 ++ 3 files changed, 299 insertions(+), 11 deletions(-) create mode 100644 token_cek_regression_test.go diff --git a/token.go b/token.go index d8379d09f..0374d2835 100644 --- a/token.go +++ b/token.go @@ -804,9 +804,10 @@ func readCekTable(r *tdsBuffer) *cekTable { var cekTable *cekTable = nil if tableSize != 0 { - mCekTable := newCekTable(tableSize) + // Allocate entries only after parsing them, not from the wire count. + mCekTable := newCekTable(0) for i := uint16(0); i < tableSize; i++ { - mCekTable.entries[i] = readCekTableEntry(r) + mCekTable.entries = append(mCekTable.entries, readCekTableEntry(r)) } cekTable = &mCekTable } @@ -814,38 +815,43 @@ func readCekTable(r *tdsBuffer) *cekTable { return cekTable } +// SQL Server permits two encrypted values per CEK during master-key rotation. +// https://learn.microsoft.com/sql/t-sql/statements/alter-column-encryption-key-transact-sql +const _MAX_CEK_VALUES = 2 + func readCekTableEntry(r *tdsBuffer) cekTableEntry { databaseId := r.int32() cekID := r.int32() cekVersion := r.int32() var cekMdVersion = make([]byte, 8) - _, err := r.Read(cekMdVersion) - if err != nil { - badStreamPanicf("unable to read cekMdVersion") + r.ReadFull(cekMdVersion) + + cekValueCount := int(r.byte()) + if cekValueCount > _MAX_CEK_VALUES { + badStreamPanic(fmt.Errorf("CEK value count %d exceeds maximum %d", cekValueCount, _MAX_CEK_VALUES)) } - cekValueCount := uint(r.byte()) // not using ucs22str because we already know the data is utf16 enc := unicode.UTF16(unicode.LittleEndian, unicode.IgnoreBOM) utf16dec := enc.NewDecoder() cekValues := make([]encryptionKeyInfo, cekValueCount) - for i := uint(0); i < cekValueCount; i++ { + for i := 0; i < cekValueCount; i++ { encryptedCekLength := r.uint16() encryptedCek := make([]byte, encryptedCekLength) r.ReadFull(encryptedCek) - keyStoreLength := r.byte() + keyStoreLength := int(r.byte()) keyStoreNameUtf16 := make([]byte, keyStoreLength*2) r.ReadFull(keyStoreNameUtf16) keyStoreName, _ := utf16dec.Bytes(keyStoreNameUtf16) - keyPathLength := r.uint16() + keyPathLength := int(r.uint16()) keyPathUtf16 := make([]byte, keyPathLength*2) r.ReadFull(keyPathUtf16) keyPath, _ := utf16dec.Bytes(keyPathUtf16) - algLength := r.byte() + algLength := int(r.byte()) algNameUtf16 := make([]byte, algLength*2) r.ReadFull(algNameUtf16) algName, _ := utf16dec.Bytes(algNameUtf16) @@ -867,7 +873,7 @@ func readCekTableEntry(r *tdsBuffer) cekTableEntry { keyId: int(cekID), keyVersion: int(cekVersion), mdVersion: cekMdVersion, - valueCount: int(cekValueCount), + valueCount: cekValueCount, cekValues: cekValues, } } diff --git a/token_cek_regression_test.go b/token_cek_regression_test.go new file mode 100644 index 000000000..8b5b66f7e --- /dev/null +++ b/token_cek_regression_test.go @@ -0,0 +1,267 @@ +package mssql + +import ( + "context" + "encoding/binary" + "fmt" + "io" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func colMetadataWithCekTable(table []byte, ordinal uint16) []byte { + stream := append([]byte{byte(tokenColMetadata), 1, 0}, table...) + stream = binary.LittleEndian.AppendUint32(stream, 0) + stream = binary.LittleEndian.AppendUint16(stream, colFlagEncrypted) + stream = append(stream, typeBigVarBin) + stream = binary.LittleEndian.AppendUint16(stream, 8000) + stream = binary.LittleEndian.AppendUint16(stream, ordinal) + stream = binary.LittleEndian.AppendUint32(stream, 0) + stream = append(stream, typeInt4, 2, 1, 1, 0) + return stream +} + +func cekEntryStream(valueCount byte, keyStore, keyPath, algorithm string) []byte { + stream := binary.LittleEndian.AppendUint32(nil, 7) + stream = binary.LittleEndian.AppendUint32(stream, 11) + stream = binary.LittleEndian.AppendUint32(stream, 2) + stream = append(stream, 1, 2, 3, 4, 5, 6, 7, 8) + stream = append(stream, valueCount) + for i := 0; i < int(valueCount); i++ { + stream = binary.LittleEndian.AppendUint16(stream, 2) + stream = append(stream, byte(i), 0xa5) + stream = append(stream, byte(len(keyStore))) + stream = append(stream, ucs2(keyStore)...) + stream = binary.LittleEndian.AppendUint16(stream, uint16(len(keyPath))) + stream = append(stream, ucs2(keyPath)...) + stream = append(stream, byte(len(algorithm))) + stream = append(stream, ucs2(algorithm)...) + } + return stream +} + +func TestReadCekTableEntry_ValueCounts(t *testing.T) { + for _, count := range []byte{0, 1, 2, 3, 255} { + t.Run(fmt.Sprint(count), func(t *testing.T) { + // Supply all advertised values so an EOF cannot stand in for the count check. + r := bufFromBytes(cekEntryStream(count, "store", "path", "RSA_OAEP")) + var entry cekTableEntry + err := recoverErr(func() { entry = readCekTableEntry(r) }) + if count > 2 { + assertStreamError(t, err) + assert.Contains(t, err.Error(), "CEK value count") + assert.Contains(t, err.Error(), "exceeds maximum 2") + assert.Equal(t, 21, r.rpos, "must reject the count before reading any key values") + return + } + if err != nil { + t.Fatal(err) + } + assert.Equal(t, 7, entry.databaseID) + assert.Equal(t, 11, entry.keyId) + assert.Equal(t, 2, entry.keyVersion) + assert.Equal(t, []byte{1, 2, 3, 4, 5, 6, 7, 8}, entry.mdVersion) + assert.Equal(t, int(count), entry.valueCount) + assert.Len(t, entry.cekValues, int(count)) + for i, value := range entry.cekValues { + assert.Equal(t, encryptionKeyInfo{ + encryptedKey: []byte{byte(i), 0xa5}, + databaseID: 7, + cekID: 11, + cekVersion: 2, + cekMdVersion: entry.mdVersion, + keyPath: "path", + keyStoreName: "store", + algorithmName: "RSA_OAEP", + }, value) + } + assert.Equal(t, r.rsize, r.rpos) + }) + } +} + +func TestReadCekTable_BogusCountAllocations(t *testing.T) { + r := bufFromBytes([]byte{0xff, 0xff}) + var errs [4]error + var before, after runtime.MemStats + runtime.ReadMemStats(&before) + for i := range errs { + r.rpos = 0 + errs[i] = recoverErr(func() { readCekTable(r) }) + } + runtime.ReadMemStats(&after) + for _, err := range errs { + assertStreamError(t, err) + } + // Four attempts amplify the count-driven allocation on both 32- and 64-bit + // platforms; the fixed parser reads EOF without allocating the advertised table. + const allocCeiling = 4 << 20 + if grew := after.TotalAlloc - before.TotalAlloc; grew > allocCeiling { + t.Fatalf("bogus CEK table counts allocated %d bytes; entries must be allocated only after parsing", grew) + } +} + +func TestReadCekTable_Empty(t *testing.T) { + r := bufFromBytes([]byte{0, 0, 0, 0, 0xa5}) + assert.Nil(t, readCekTable(r)) + assert.Nil(t, readCekTable(r)) + assert.Equal(t, byte(0xa5), r.byte()) +} + +func TestReadCekTable_PacketBoundaries(t *testing.T) { + table := []byte{2, 0} + table = append(table, cekEntryStream(1, "store", "path", "RSA_OAEP")...) + table = append(table, cekEntryStream(2, "store", "path", "RSA_OAEP")...) + stream := append(append([]byte{}, table...), table...) + stream = append(stream, 0xa5) + + for _, chunk := range []int{0, 1, 3, 13} { + t.Run(fmt.Sprint(chunk), func(t *testing.T) { + packets, ok := frameReplyPackets(stream, chunk) + if !ok { + t.Fatal("failed to frame CEK tables") + } + sess := newFuzzSession(packets) + defer sess.buf.bufClose() + if _, err := sess.buf.BeginRead(); err != nil { + t.Fatal(err) + } + for attempt := 0; attempt < 2; attempt++ { + var got *cekTable + if err := recoverErr(func() { got = readCekTable(sess.buf) }); err != nil { + t.Fatal(err) + } + if got == nil || len(got.entries) != 2 { + t.Fatalf("expected two CEK entries, got %#v", got) + } + for i, entry := range got.entries { + assert.Equal(t, i+1, entry.valueCount) + assert.Equal(t, []byte{1, 2, 3, 4, 5, 6, 7, 8}, entry.mdVersion) + assert.Len(t, entry.cekValues, i+1) + for j, value := range entry.cekValues { + assert.Equal(t, []byte{byte(j), 0xa5}, value.encryptedKey) + assert.Equal(t, "store", value.keyStoreName) + assert.Equal(t, "path", value.keyPath) + assert.Equal(t, "RSA_OAEP", value.algorithmName) + } + } + } + assert.Equal(t, byte(0xa5), sess.buf.byte()) + _, err := sess.buf.ReadByte() + assert.ErrorIs(t, err, io.EOF) + }) + } +} + +func TestReadCekTableEntry_Lengths(t *testing.T) { + cases := []struct { + name string + keyStore string + keyPath string + algorithm string + }{ + {"provider", strings.Repeat("s", 128), "path", "RSA_OAEP"}, + {"path", "store", strings.Repeat("p", 32768), "RSA_OAEP"}, + {"algorithm", "store", "path", strings.Repeat("a", 128)}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + stream := cekEntryStream(1, tc.keyStore, tc.keyPath, tc.algorithm) + packets, ok := frameReplyPackets(stream, 0) + if !ok { + t.Fatal("failed to frame CEK entry") + } + sess := newFuzzSession(packets) + defer sess.buf.bufClose() + if _, err := sess.buf.BeginRead(); err != nil { + t.Fatal(err) + } + var entry cekTableEntry + if err := recoverErr(func() { entry = readCekTableEntry(sess.buf) }); err != nil { + t.Fatal(err) + } + if len(entry.cekValues) != 1 { + t.Fatalf("expected one CEK value, got %d", len(entry.cekValues)) + } + value := entry.cekValues[0] + assert.Equal(t, tc.keyStore, value.keyStoreName) + assert.Equal(t, tc.keyPath, value.keyPath) + assert.Equal(t, tc.algorithm, value.algorithmName) + _, err := sess.buf.ReadByte() + assert.ErrorIs(t, err, io.EOF) + }) + } +} + +func TestReadCekTableEntry_Truncated(t *testing.T) { + stream := cekEntryStream(1, "store", "path", "RSA_OAEP") + for end := 0; end < len(stream); end++ { + t.Run(fmt.Sprint(end), func(t *testing.T) { + err := recoverErr(func() { readCekTableEntry(bufFromBytes(stream[:end])) }) + assertStreamError(t, err) + }) + } +} + +func TestParseColMetadata72_CekTable(t *testing.T) { + table := []byte{2, 0} + table = append(table, cekEntryStream(1, "store", "path", "RSA_OAEP")...) + second := cekEntryStream(2, "store", "path", "RSA_OAEP") + binary.LittleEndian.PutUint32(second[4:8], 12) + table = append(table, second...) + stream := colMetadataWithCekTable(table, 1) + columns := parseColMetadata72(bufFromBytes(stream[1:]), &tdsSession{alwaysEncrypted: true}) + if len(columns) != 1 || columns[0].cryptoMeta == nil || columns[0].cryptoMeta.entry == nil { + t.Fatal("expected one encrypted column with CEK metadata") + } + cm := columns[0].cryptoMeta + assert.Equal(t, uint16(1), cm.ordinal) + assert.Equal(t, 12, cm.entry.keyId) + assert.Equal(t, 2, cm.entry.valueCount) + assert.Equal(t, byte(typeInt4), cm.typeInfo.TypeId) + assert.True(t, columns[0].isEncrypted()) +} + +func TestProcessSingleResponse_CekAllocations(t *testing.T) { + cases := map[string][]byte{ + "bogus table count": {byte(tokenColMetadata), 0, 0, 0xff, 0xff}, + "bogus value count": colMetadataWithCekTable( + append([]byte{0xff, 0xff}, cekEntryStream(255, "store", "path", "RSA_OAEP")...), 0), + } + for name, stream := range cases { + t.Run(name, func(t *testing.T) { + for _, chunk := range []int{0, 1, 3} { + tokens, _, sawError, framed := drainSingleResponseWithEncryption(stream, chunk, true, true) + if !framed || !sawError { + t.Fatalf("expected a malformed encrypted response, chunk=%d", chunk) + } + assert.Contains(t, tokens, "error:mssql.StreamError") + } + }) + } +} + +func TestProcessSingleResponse_CekRotation(t *testing.T) { + table := append([]byte{1, 0}, cekEntryStream(2, "store", "path", "RSA_OAEP")...) + stream := append(colMetadataWithCekTable(table, 0), doneToken(tokenDone, 0)...) + for _, chunk := range []int{0, 1, 3} { + tokens, _, sawError, framed := drainSingleResponseWithEncryption(stream, chunk, true, true) + if !framed || sawError { + t.Fatalf("valid encrypted metadata failed with chunk=%d: %v", chunk, tokens) + } + assert.Contains(t, tokens, fmt.Sprintf("cols[{name=%q type=%d usertype=0 flags=%d}]", "", typeBigVarBin, colFlagEncrypted)) + assert.Contains(t, tokens, "done{status=0 curcmd=0 rows=0 errs=[]}") + } +} + +func TestCekStreamErrorMarksConnectionBad(t *testing.T) { + r := bufFromBytes(cekEntryStream(255, "store", "path", "RSA_OAEP")) + err := recoverErr(func() { readCekTableEntry(r) }) + assertStreamError(t, err) + conn := &Conn{connectionGood: true} + assert.Equal(t, err, conn.checkBadConn(context.Background(), err, false)) + assert.False(t, conn.connectionGood) +} diff --git a/token_fuzz_test.go b/token_fuzz_test.go index d20e4f962..b01958104 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -209,11 +209,16 @@ func normalizeToken(tok tokenStruct) string { // processSingleResponse installs its own recover(), so a malformed stream // surfaces as an error token here rather than crashing the harness. func drainSingleResponse(stream []byte, chunk int, collect bool) (tokens []string, dbState string, sawError bool, framed bool) { + return drainSingleResponseWithEncryption(stream, chunk, collect, false) +} + +func drainSingleResponseWithEncryption(stream []byte, chunk int, collect, alwaysEncrypted bool) (tokens []string, dbState string, sawError bool, framed bool) { packets, ok := frameReplyPackets(stream, chunk) if !ok { return nil, "", false, false } sess := newFuzzSession(packets) + sess.alwaysEncrypted = alwaysEncrypted defer sess.buf.bufClose() ch := make(chan tokenStruct, 5) @@ -594,6 +599,15 @@ func FuzzProcessSingleResponse(f *testing.F) { 13, 0, 0, 0, // dataOffset }, uint16(0)) + // CEK metadata is parsed only when Always Encrypted was negotiated. + f.Add([]byte{byte(tokenColMetadata), 0, 0, 0xff, 0xff}, uint16(0)) + bogusCekTable := append([]byte{0xff, 0xff}, cekEntryStream(255, "store", "path", "RSA_OAEP")...) + f.Add(colMetadataWithCekTable(bogusCekTable, 0), uint16(0)) + validCekTable := append([]byte{1, 0}, cekEntryStream(2, "store", "path", "RSA_OAEP")...) + encryptedMetadata := append(colMetadataWithCekTable(validCekTable, 0), doneToken(tokenDone, 0)...) + f.Add(encryptedMetadata, uint16(0)) + f.Add(encryptedMetadata, uint16(3)) + f.Fuzz(func(t *testing.T, stream []byte, frag uint16) { // Bound input size to keep framing and allocations reasonable. A TDS // packet length is a uint16, and the read buffer is 32 KiB, so very @@ -611,5 +625,6 @@ func FuzzProcessSingleResponse(f *testing.F) { // are intentionally unused beyond confirming completion, so collect is // false to avoid per-token allocations in the hot fuzzing path. _, _, _, _ = drainSingleResponse(stream, chunk, false) + _, _, _, _ = drainSingleResponseWithEncryption(stream, chunk, false, true) }) } From 30488dce0a68c479446b9b95857fca6c6cc5de9d Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Thu, 10 Sep 2026 18:24:19 -0700 Subject: [PATCH 35/38] fix: defer value buffers and bound cumulative PLP payloads Allocate reusable column buffers from value lengths instead of metadata maxima. Stream PLP values without reserving advertised sizes, and check every chunk against the remaining per-value budget. Cover repeated values, empty PLP columns, truncation, encrypted data, packet seams, and malformed response seeds. Fixes #420 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token_fuzz_test.go | 17 ++ types.go | 87 +++++---- types_alloc_regression_test.go | 320 +++++++++++++++++++++++++++++++++ 3 files changed, 387 insertions(+), 37 deletions(-) create mode 100644 types_alloc_regression_test.go diff --git a/token_fuzz_test.go b/token_fuzz_test.go index b01958104..d745b309e 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -608,6 +608,23 @@ func FuzzProcessSingleResponse(f *testing.F) { f.Add(encryptedMetadata, uint16(0)) f.Add(encryptedMetadata, uint16(3)) + wideMetadata := colMetadataVarBinary(64, 0xfffe) + f.Add(wideMetadata, uint16(0)) + truncatedMetadata := append([]byte{}, wideMetadata...) + binary.LittleEndian.PutUint16(truncatedMetadata[1:3], 0xfffe) + f.Add(truncatedMetadata, uint16(3)) + + emptyPLP := append(colMetadataVarBinary(1, 0xffff), byte(tokenRow)) + emptyPLP = append(emptyPLP, plpChunks(_MAX_PLP_LEN)...) + emptyPLP = append(emptyPLP, doneToken(tokenDone, 0)...) + f.Add(emptyPLP, uint16(0)) + for _, chunkSize := range []uint32{_MAX_PLP_LEN + 1, 0xffffffff} { + stream := append(colMetadataVarBinary(1, 0xffff), byte(tokenRow)) + stream = binary.LittleEndian.AppendUint64(stream, _UNKNOWN_PLP_LEN) + stream = binary.LittleEndian.AppendUint32(stream, chunkSize) + f.Add(stream, uint16(0)) + } + f.Fuzz(func(t *testing.T, stream []byte, frag uint16) { // Bound input size to keep framing and allocations reasonable. A TDS // packet length is a uint16, and the read buffer is 32 KiB, so very diff --git a/types.go b/types.go index 3b2b1894b..82371cd0f 100644 --- a/types.go +++ b/types.go @@ -120,6 +120,21 @@ type typeInfo struct { Writer func(w io.Writer, ti typeInfo, buf []byte, encoding msdsn.EncodeParameters) (err error) } +func (ti *typeInfo) getBuffer(size int) []byte { + if size > ti.Size || len(ti.Buffer) < size { + ti.growBuffer(size) + } + return ti.Buffer[:size] +} + +// Keep allocation and error construction out of the inlined buffer-reuse path. +func (ti *typeInfo) growBuffer(size int) { + if size > ti.Size { + badStreamPanic(fmt.Errorf("value length %d exceeds declared type size %d", size, ti.Size)) + } + ti.Buffer = make([]byte, size) +} + // Common Language Runtime (CLR) Instances // http://msdn.microsoft.com/en-us/library/dd357962.aspx type udtInfo struct { @@ -158,7 +173,6 @@ func readTypeInfo(r *tdsBuffer, typeId byte, c *cryptoMetadata, encoding msdsn.E res.Size = 8 } res.Reader = readFixedType - res.Buffer = make([]byte, res.Size) default: // all others are VARLENTYPE readVarLen(&res, r, c, encoding) } @@ -360,8 +374,8 @@ func nanosToThreeHundredthsOfASecond(ns int) int { } func readFixedType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msdsn.EncodeParameters) interface{} { - r.ReadFull(ti.Buffer) - buf := ti.Buffer + buf := ti.getBuffer(ti.Size) + r.ReadFull(buf) loc := encoding.GetTimezone() switch ti.TypeId { case typeNull: @@ -405,8 +419,8 @@ func readByteLenTypeWithEncoding(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, return nil } loc := encoding.GetTimezone() - r.ReadFull(ti.Buffer[:size]) - buf := ti.Buffer[:size] + buf := ti.getBuffer(int(size)) + r.ReadFull(buf) switch ti.TypeId { case typeDateN: if len(buf) != 3 { @@ -530,8 +544,8 @@ func readShortLenType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding ms if size == 0xffff { return nil } - r.ReadFull(ti.Buffer[:size]) - buf := ti.Buffer[:size] + buf := ti.getBuffer(int(size)) + r.ReadFull(buf) switch ti.TypeId { case typeBigVarChar, typeBigChar: return decodeChar(ti.Collation, buf) @@ -780,36 +794,40 @@ func readVariantTypeWithEncoding(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, // partially length prefixed stream // http://msdn.microsoft.com/en-us/library/dd340469.aspx +// maxLen bounds the entire value, including all chunks with an unknown total. +func readPLPBytes(r *tdsBuffer, maxLen uint64) []byte { + size := r.uint64() + if size == _PLP_NULL { + return nil + } + if size != _UNKNOWN_PLP_LEN && size > maxLen { + badStreamPanic(fmt.Errorf("PLP length %d exceeds the maximum LOB size of %d bytes", size, maxLen)) + } + // Even a bounded capacity hint is amplified by many empty PLP columns. + buf := bytes.NewBuffer([]byte{}) + for { + chunksize := r.uint32() + if chunksize == 0 { + break + } + remaining := maxLen - uint64(buf.Len()) + if uint64(chunksize) > remaining { + badStreamPanic(fmt.Errorf("PLP chunk length %d exceeds the remaining LOB size of %d bytes", chunksize, remaining)) + } + if _, err := io.CopyN(buf, r, int64(chunksize)); err != nil { + badStreamPanic(fmt.Errorf("reading PLP value failed: %w", err)) + } + } + return buf.Bytes() +} + func readPLPType(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msdsn.EncodeParameters) interface{} { var bytesToDecode []byte if c == nil { - size := r.uint64() - var buf *bytes.Buffer - switch size { - case _PLP_NULL: - // null + bytesToDecode = readPLPBytes(r, _MAX_PLP_LEN) + if bytesToDecode == nil { return nil - case _UNKNOWN_PLP_LEN: - // size unknown - buf = bytes.NewBuffer(make([]byte, 0, 1000)) - default: - // The advertised size is untrusted, so reject anything a real server - // cannot produce before using it as an allocation size. - if size > _MAX_PLP_LEN { - badStreamPanicf("PLP length %d exceeds the maximum LOB size of %d bytes", size, uint64(_MAX_PLP_LEN)) - } - buf = bytes.NewBuffer(make([]byte, 0, size)) - } - for { - chunksize := r.uint32() - if chunksize == 0 { - break - } - if _, err := io.CopyN(buf, r, int64(chunksize)); err != nil { - badStreamPanicf("Reading PLP type failed: %s", err.Error()) - } } - bytesToDecode = buf.Bytes() } else { bytesToDecode = r.rbuf } @@ -857,7 +875,6 @@ func readVarLen(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msdsn.En case typeDateN: ti.Size = 3 ti.Reader = readByteLenTypeWithEncoding - ti.Buffer = make([]byte, ti.Size) case typeTimeN, typeDateTime2N, typeDateTimeOffsetN: ti.Scale = r.byte() switch ti.Scale { @@ -877,14 +894,12 @@ func readVarLen(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msdsn.En ti.Size += 5 } ti.Reader = readByteLenTypeWithEncoding - ti.Buffer = make([]byte, ti.Size) case typeGuid, typeIntN, typeDecimal, typeNumeric, typeBitN, typeDecimalN, typeNumericN, typeFltN, typeMoneyN, typeDateTimeN, typeChar, typeVarChar, typeBinary, typeVarBinary: // byle len types ti.Size = int(r.byte()) - ti.Buffer = make([]byte, ti.Size) switch ti.TypeId { case typeDecimal, typeNumeric, typeDecimalN, typeNumericN: ti.Prec = r.byte() @@ -909,7 +924,6 @@ func readVarLen(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msdsn.En ti.UdtInfo.TypeName = r.BVarChar() ti.UdtInfo.AssemblyQualifiedName = r.UsVarChar() - ti.Buffer = make([]byte, ti.Size) ti.Reader = readPLPType case typeBigVarBin, typeBigVarChar, typeBigBinary, typeBigChar, typeNVarChar, typeNChar: @@ -922,7 +936,6 @@ func readVarLen(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, encoding msdsn.En if ti.Size == 0xffff { ti.Reader = readPLPType } else { - ti.Buffer = make([]byte, ti.Size) ti.Reader = readShortLenType } case typeText, typeImage, typeNText, typeVariant: diff --git a/types_alloc_regression_test.go b/types_alloc_regression_test.go new file mode 100644 index 000000000..5258f0d9e --- /dev/null +++ b/types_alloc_regression_test.go @@ -0,0 +1,320 @@ +package mssql + +import ( + "bytes" + "context" + "encoding/binary" + "fmt" + "io" + "strings" + "testing" + + "github.com/microsoft/go-mssqldb/msdsn" + "github.com/stretchr/testify/assert" +) + +func colMetadataVarBinary(count, size uint16) []byte { + stream := binary.LittleEndian.AppendUint16([]byte{byte(tokenColMetadata)}, count) + for i := uint16(0); i < count; i++ { + stream = binary.LittleEndian.AppendUint32(stream, 0) + stream = binary.LittleEndian.AppendUint16(stream, 0) + stream = append(stream, typeBigVarBin) + stream = binary.LittleEndian.AppendUint16(stream, size) + stream = append(stream, 0) + } + return stream +} + +func plpChunks(size uint64, chunks ...[]byte) []byte { + stream := binary.LittleEndian.AppendUint64(nil, size) + if size == _PLP_NULL { + return stream + } + for _, chunk := range chunks { + stream = binary.LittleEndian.AppendUint32(stream, uint32(len(chunk))) + stream = append(stream, chunk...) + } + return binary.LittleEndian.AppendUint32(stream, 0) +} + +func TestReadTypeInfo_DefersValueBuffers(t *testing.T) { + cases := []struct { + name string + typeID byte + metadata []byte + size int + }{ + {"fixed", typeInt4, nil, 4}, + {"byte", typeVarBinary, []byte{255}, 255}, + {"date", typeDateN, nil, 3}, + {"time", typeTimeN, []byte{7}, 5}, + {"decimal", typeDecimalN, []byte{17, 38, 0}, 17}, + {"binary", typeBigVarBin, []byte{0xfe, 0xff}, 65534}, + {"unicode", typeNVarChar, []byte{0xfe, 0xff, 0, 0, 0, 0, 0}, 65534}, + {"udt", typeUdt, []byte{0xff, 0xff, 0, 0, 0, 0, 0}, 65535}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ti := readTypeInfo(bufFromBytes(tc.metadata), tc.typeID, nil, msdsn.EncodeParameters{}) + assert.Equal(t, tc.size, ti.Size) + assert.Zero(t, cap(ti.Buffer), "metadata must not reserve storage for column values") + assert.NotNil(t, ti.Reader) + }) + } +} + +func TestParseColMetadata72_DefersValueBuffers(t *testing.T) { + // Scale the reported 65534-column input down to 64 columns so the unfixed + // regression allocates only 4 MiB, not 4 GiB, while exercising the same path. + const count = 64 + stream := colMetadataVarBinary(count, 0xfffe) + columns := parseColMetadata72(bufFromBytes(stream[1:]), &tdsSession{}) + if len(columns) != count { + t.Fatalf("got %d columns, want %d", len(columns), count) + } + for i, column := range columns { + assert.Equal(t, 65534, column.ti.Size) + if cap(column.ti.Buffer) != 0 { + t.Fatalf("column %d reserved %d bytes before any row was read", i, cap(column.ti.Buffer)) + } + } +} + +func TestReadPLPType_LargeHintDoesNotPreallocate(t *testing.T) { + for _, size := range []uint64{16 << 20, _MAX_PLP_LEN} { + t.Run(fmt.Sprint(size), func(t *testing.T) { + r := bufFromBytes(plpChunks(size)) + ti := typeInfo{TypeId: typeBigVarBin} + value := readPLPType(&ti, r, nil, msdsn.EncodeParameters{}) + data, ok := value.([]byte) + if !ok || data == nil || len(data) != 0 { + t.Fatalf("expected a non-nil empty binary value, got %T", value) + } + if cap(data) != 0 { + t.Fatalf("empty PLP value retained %d bytes from its advertised length", cap(data)) + } + _, err := r.ReadByte() + assert.ErrorIs(t, err, io.EOF) + }) + } +} + +func TestValueReaders_LazyBufferReuse(t *testing.T) { + shortValues := []byte{3, 0, 'a', 'b', 'c', 5, 0, 'd', 'e', 'f', 'g', 'h', 1, 0, 'i', 0, 0, 0xff, 0xff} + cases := []struct { + name string + typeID byte + metadata []byte + stream []byte + want []interface{} + maxBuffer int + }{ + {"fixed", typeInt4, nil, []byte{41, 0, 0, 0, 42, 0, 0, 0}, []interface{}{int64(41), int64(42)}, 4}, + {"byte", typeVarBinary, []byte{255}, []byte{3, 'a', 'b', 'c', 5, 'd', 'e', 'f', 'g', 'h', 1, 'i', 0}, + []interface{}{[]byte("abc"), []byte("defgh"), []byte("i"), nil}, 5}, + {"short", typeBigVarBin, []byte{0xfe, 0xff}, shortValues, + []interface{}{[]byte("abc"), []byte("defgh"), []byte("i"), []byte{}, nil}, 5}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ti := readTypeInfo(bufFromBytes(tc.metadata), tc.typeID, nil, msdsn.EncodeParameters{}) + stream := append(append([]byte{}, tc.stream...), tc.stream...) + r := bufFromBytes(stream) + for attempt := 0; attempt < 2; attempt++ { + var got []interface{} + for range tc.want { + got = append(got, ti.Reader(&ti, r, nil, msdsn.EncodeParameters{})) + } + // Compare after every value is read to detect reuse corrupting earlier rows. + assert.Equal(t, tc.want, got) + assert.Equal(t, tc.maxBuffer, cap(ti.Buffer)) + } + assert.Equal(t, len(stream), r.rpos) + }) + } +} + +func TestValueReaders_RejectOversizedValues(t *testing.T) { + for _, typeID := range []byte{typeVarBinary, typeBigVarBin} { + t.Run(fmt.Sprint(typeID), func(t *testing.T) { + metadata := []byte{4} + stream := []byte{5, 1, 2, 3, 4, 5} + if typeID == typeBigVarBin { + metadata = []byte{4, 0} + stream = []byte{5, 0, 1, 2, 3, 4, 5} + } + ti := readTypeInfo(bufFromBytes(metadata), typeID, nil, msdsn.EncodeParameters{}) + err := recoverErr(func() { ti.Reader(&ti, bufFromBytes(stream), nil, msdsn.EncodeParameters{}) }) + assertStreamError(t, err) + assert.Contains(t, err.Error(), "exceeds declared type size") + assert.Zero(t, cap(ti.Buffer)) + }) + } +} + +func TestReadPLPBytes_CumulativeLimit(t *testing.T) { + const maxLen = 16 + first := []byte("12345678") + second := []byte("abcdefgh") + for _, size := range []uint64{0, maxLen, _UNKNOWN_PLP_LEN} { + t.Run(fmt.Sprint(size), func(t *testing.T) { + // Exercise the production chunk loop with a small limit rather than + // allocating the protocol maximum of 2 GiB. + stream := plpChunks(size, first, second) + r := bufFromBytes(append(append([]byte{}, stream...), stream...)) + for attempt := 0; attempt < 2; attempt++ { + assert.Equal(t, []byte("12345678abcdefgh"), readPLPBytes(r, maxLen)) + } + _, err := r.ReadByte() + assert.ErrorIs(t, err, io.EOF) + + r = bufFromBytes(plpChunks(size, first, second, []byte("!"))) + err = recoverErr(func() { readPLPBytes(r, maxLen) }) + assertStreamError(t, err) + assert.Contains(t, err.Error(), "remaining LOB size") + assert.Equal(t, 36, r.rpos, "reject the final chunk before reading its payload") + }) + } +} + +func TestReadPLPBytes_LengthsAndTruncation(t *testing.T) { + const maxLen = 16 + cases := []struct { + name string + stream []byte + }{ + {"oversized hint", plpChunks(maxLen + 1)}, + {"oversized chunk", binary.LittleEndian.AppendUint32( + binary.LittleEndian.AppendUint64(nil, _UNKNOWN_PLP_LEN), 0xffffffff)}, + {"truncated chunk", append(binary.LittleEndian.AppendUint32( + binary.LittleEndian.AppendUint64(nil, 4), 4), 1, 2)}, + {"missing terminator", plpChunks(2, []byte{1, 2})[:14]}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := recoverErr(func() { readPLPBytes(bufFromBytes(tc.stream), maxLen) }) + assertStreamError(t, err) + }) + } + for _, size := range []uint64{0, _UNKNOWN_PLP_LEN} { + value := readPLPBytes(bufFromBytes(plpChunks(size)), maxLen) + assert.NotNil(t, value) + assert.Empty(t, value) + } + assert.Nil(t, readPLPBytes(bufFromBytes(plpChunks(_PLP_NULL)), maxLen)) +} + +func TestReadPLPBytes_PacketBoundaries(t *testing.T) { + stream := plpChunks(_UNKNOWN_PLP_LEN, []byte("abc"), []byte("defgh")) + for chunk := 1; chunk < len(stream); chunk++ { + t.Run(fmt.Sprint(chunk), func(t *testing.T) { + packets, ok := frameReplyPackets(stream, chunk) + if !ok { + t.Fatal("failed to frame PLP data") + } + sess := newFuzzSession(packets) + defer sess.buf.bufClose() + if _, err := sess.buf.BeginRead(); err != nil { + t.Fatal(err) + } + assert.Equal(t, []byte("abcdefgh"), readPLPBytes(sess.buf, 16)) + _, err := sess.buf.ReadByte() + assert.ErrorIs(t, err, io.EOF) + }) + } +} + +func TestValueReaders_DecryptedBuffers(t *testing.T) { + payload := []byte{1, 2, 3} + for _, size := range []uint16{3, 0xffff} { + t.Run(fmt.Sprint(size), func(t *testing.T) { + metadata := binary.LittleEndian.AppendUint16(nil, size) + ti := readTypeInfo(bufFromBytes(metadata), typeBigVarBin, nil, msdsn.EncodeParameters{}) + ti.Buffer = append([]byte{}, payload...) + r := &tdsBuffer{rbuf: payload, rsize: len(payload), final: true} + got := ti.Reader(&ti, r, &cryptoMetadata{}, msdsn.EncodeParameters{}) + assert.Equal(t, payload, got) + }) + } +} + +func TestProcessSingleResponse_DeferredMetadataAndPLP(t *testing.T) { + stream := colMetadataVarBinary(1, 0xffff) + stream = append(stream, byte(tokenRow)) + stream = append(stream, plpChunks(_MAX_PLP_LEN)...) + stream = append(stream, doneToken(tokenDone, 0)...) + for _, chunk := range []int{0, 1, 3} { + _, _, sawError, framed := drainSingleResponse(stream, chunk, true) + if !framed || sawError { + t.Fatalf("empty PLP with a maximum-length hint failed, chunk=%d", chunk) + } + } + + stream = colMetadataVarBinary(64, 0xfffe) + stream = append(stream, byte(tokenRow)) + for i := 0; i < 64; i++ { + stream = append(stream, 1, 0, byte(i)) + } + stream = append(stream, doneToken(tokenDone, 0)...) + _, _, sawError, framed := drainSingleResponse(stream, 3, true) + if !framed || sawError { + t.Fatal("small values with large metadata maxima failed") + } + + metadata := colMetadataVarBinary(1, 0xffff) + for _, chunkSize := range []uint32{_MAX_PLP_LEN + 1, 0xffffffff} { + stream = append(append([]byte{}, metadata...), byte(tokenRow)) + stream = binary.LittleEndian.AppendUint64(stream, _UNKNOWN_PLP_LEN) + stream = binary.LittleEndian.AppendUint32(stream, chunkSize) + tokens, _, sawError, framed := drainSingleResponse(stream, 1, true) + if !framed || !sawError { + t.Fatal("oversized PLP chunk did not fail") + } + assert.Contains(t, tokens, "error:mssql.StreamError") + assert.Contains(t, strings.Join(tokens, "\n"), "remaining LOB size") + } +} + +func TestReadPLPType_Null(t *testing.T) { + for _, typeID := range []byte{typeBigVarBin, typeImage, typeNVarChar, typeXml, typeUdt} { + ti := typeInfo{TypeId: typeID} + assert.Nil(t, readPLPType(&ti, bufFromBytes(plpChunks(_PLP_NULL)), nil, msdsn.EncodeParameters{})) + } +} + +func TestReadPLPType_ChunkValues(t *testing.T) { + payload := bytes.Repeat([]byte("a"), 70<<10) + stream := plpChunks(uint64(len(payload)), payload[:1000], payload[1000:]) + packets, ok := frameReplyPackets(stream, 1024) + if !ok { + t.Fatal("failed to frame PLP value") + } + sess := newFuzzSession(packets) + defer sess.buf.bufClose() + if _, err := sess.buf.BeginRead(); err != nil { + t.Fatal(err) + } + ti := typeInfo{TypeId: typeBigVarBin} + assert.Equal(t, payload, readPLPType(&ti, sess.buf, nil, msdsn.EncodeParameters{})) +} + +func TestParseRow_EmptyPLPValuesDoNotReserveHints(t *testing.T) { + const count = 64 + metadata := colMetadataVarBinary(count, 0xffff) + columns := parseColMetadata72(bufFromBytes(metadata[1:]), &tdsSession{}) + stream := bytes.Repeat(plpChunks(_MAX_PLP_LEN), count*2) + r := bufFromBytes(stream) + row := make([]interface{}, count) + for attempt := 0; attempt < 2; attempt++ { + if err := parseRow(context.Background(), r, &tdsSession{}, columns, row); err != nil { + t.Fatal(err) + } + for i, value := range row { + data, ok := value.([]byte) + if !ok || data == nil || len(data) != 0 || cap(data) != 0 { + t.Fatalf("empty PLP column %d must not retain memory from its length hint", i) + } + } + } + assert.Equal(t, len(stream), r.rpos) +} From 22e913de4855a75b07cf1f7f550de7e9bf236ab1 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Tue, 22 Sep 2026 20:23:14 -0700 Subject: [PATCH 36/38] fix: validate sql_variant widths before reading values Reject invalid property counts, fixed and scale-dependent payload widths, decimal lengths, and undersized headers as StreamError before reading outside a variant. Preserve valid values across consecutive reads and packet boundaries, and add ROW, NBCROW, return-value and fuzz regressions. Fixes #420 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token_fuzz_test.go | 10 ++ types.go | 90 ++++++++---- types_variant_test.go | 328 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 404 insertions(+), 24 deletions(-) create mode 100644 types_variant_test.go diff --git a/token_fuzz_test.go b/token_fuzz_test.go index d745b309e..bb16a3129 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -625,6 +625,16 @@ func FuzzProcessSingleResponse(f *testing.F) { f.Add(stream, uint16(0)) } + for _, value := range [][]byte{ + variantStream(typeInt8, nil, []byte{0x12}), + variantStream(typeTimeN, nil, []byte{7, 0, 0, 0, 0, 0}), + variantStream(typeDecimalN, []byte{38, 0}, make([]byte, 21)), + variantStream(typeInt8, nil, binary.LittleEndian.AppendUint64(nil, 42)), + } { + f.Add(variantResponse(value), uint16(0)) + f.Add(variantResponse(value), uint16(3)) + } + f.Fuzz(func(t *testing.T, stream []byte, frag uint16) { // Bound input size to keep framing and allocations reasonable. A TDS // packet length is a uint16, and the read buffer is 32 KiB, so very diff --git a/types.go b/types.go index 82371cd0f..7f2999f7d 100644 --- a/types.go +++ b/types.go @@ -699,6 +699,9 @@ func readVariantTypeWithEncoding(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, if size == 0 { return nil } + if size < 2 { + badStreamPanic(fmt.Errorf("sql_variant data length %d is invalid", size)) + } vartype := r.byte() propbytes := int32(r.byte()) // size-2-propbytes is the trailing data length and is used below as an @@ -706,88 +709,127 @@ func readVariantTypeWithEncoding(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, // so reject an underflowed (negative) or implausibly large value before any // make() to avoid an OOM DoS (issue #420). A sql_variant tops out at ~8 KB // on the wire, so it is bounded far below the LOB ceiling. - if datalen := size - 2 - propbytes; datalen < 0 || datalen > _MAX_VARIANT_LEN { + datalen := size - 2 - propbytes + if datalen < 0 || datalen > _MAX_VARIANT_LEN { badStreamPanic(fmt.Errorf("sql_variant data length %d is invalid", datalen)) } + // Each read must fit the variant, not consume bytes from the next value. + checkLengths := func(wantProps, wantData int32) { + if propbytes != wantProps { + badStreamPanic(fmt.Errorf("sql_variant type 0x%x has %d property bytes, want %d", vartype, propbytes, wantProps)) + } + if datalen != wantData { + badStreamPanic(fmt.Errorf("sql_variant type 0x%x has data length %d, want %d", vartype, datalen, wantData)) + } + } switch vartype { case typeGuid: - buf := make([]byte, size-2-propbytes) + checkLengths(0, 16) + buf := make([]byte, datalen) r.ReadFull(buf) return decodeGuid(buf, encoding) case typeBit: + checkLengths(0, 1) return r.byte() != 0 case typeInt1: + checkLengths(0, 1) return int64(r.byte()) case typeInt2: + checkLengths(0, 2) return int64(int16(r.uint16())) case typeInt4: + checkLengths(0, 4) return int64(r.int32()) case typeInt8: + checkLengths(0, 8) return int64(r.uint64()) case typeDateTime: - buf := make([]byte, size-2-propbytes) + checkLengths(0, 8) + buf := make([]byte, datalen) r.ReadFull(buf) return decodeDateTime(buf, loc) case typeDateTim4: - buf := make([]byte, size-2-propbytes) + checkLengths(0, 4) + buf := make([]byte, datalen) r.ReadFull(buf) return decodeDateTim4(buf, loc) case typeFlt4: + checkLengths(0, 4) return float64(math.Float32frombits(r.uint32())) case typeFlt8: + checkLengths(0, 8) return math.Float64frombits(r.uint64()) case typeMoney4: - buf := make([]byte, size-2-propbytes) + checkLengths(0, 4) + buf := make([]byte, datalen) r.ReadFull(buf) return decodeMoney4(buf) case typeMoney: - buf := make([]byte, size-2-propbytes) + checkLengths(0, 8) + buf := make([]byte, datalen) r.ReadFull(buf) return decodeMoney(buf) case typeDateN: - buf := make([]byte, size-2-propbytes) + checkLengths(0, 3) + buf := make([]byte, datalen) r.ReadFull(buf) return decodeDate(buf, loc) - case typeTimeN: - scale := r.byte() - buf := make([]byte, size-2-propbytes) - r.ReadFull(buf) - return decodeTime(scale, buf, loc) - case typeDateTime2N: - scale := r.byte() - buf := make([]byte, size-2-propbytes) - r.ReadFull(buf) - return decodeDateTime2(scale, buf, loc) - case typeDateTimeOffsetN: + case typeTimeN, typeDateTime2N, typeDateTimeOffsetN: + checkLengths(1, datalen) scale := r.byte() - buf := make([]byte, size-2-propbytes) + if scale > 7 { + badStreamPanic(fmt.Errorf("sql_variant type 0x%x has invalid scale %d", vartype, scale)) + } + wantData := int32(calcTimeSize(int(scale))) + if vartype == typeDateTime2N { + wantData += 3 + } else if vartype == typeDateTimeOffsetN { + wantData += 5 + } + checkLengths(1, wantData) + buf := make([]byte, datalen) r.ReadFull(buf) + if vartype == typeTimeN { + return decodeTime(scale, buf, loc) + } + if vartype == typeDateTime2N { + return decodeDateTime2(scale, buf, loc) + } return decodeDateTimeOffset(scale, buf) case typeBigVarBin, typeBigBinary: + checkLengths(2, datalen) r.uint16() // max length, ignoring - buf := make([]byte, size-2-propbytes) + buf := make([]byte, datalen) r.ReadFull(buf) return buf case typeDecimalN, typeNumericN: + checkLengths(2, datalen) + switch datalen { + case 5, 9, 13, 17: + default: + badStreamPanic(fmt.Errorf("sql_variant type 0x%x has invalid data length %d", vartype, datalen)) + } prec := r.byte() scale := r.byte() - buf := make([]byte, size-2-propbytes) + buf := make([]byte, datalen) r.ReadFull(buf) return decodeDecimal(prec, scale, buf) case typeBigVarChar, typeBigChar: + checkLengths(7, datalen) col := readCollation(r) r.uint16() // max length, ignoring - buf := make([]byte, size-2-propbytes) + buf := make([]byte, datalen) r.ReadFull(buf) return decodeChar(col, buf) case typeNVarChar, typeNChar: + checkLengths(7, datalen) _ = readCollation(r) r.uint16() // max length, ignoring - buf := make([]byte, size-2-propbytes) + buf := make([]byte, datalen) r.ReadFull(buf) return decodeNChar(buf) default: - badStreamPanicf("Invalid variant typeid") + badStreamPanic(fmt.Errorf("invalid sql_variant type 0x%x", vartype)) } panic("shoulnd't get here") } diff --git a/types_variant_test.go b/types_variant_test.go new file mode 100644 index 000000000..bc82aec73 --- /dev/null +++ b/types_variant_test.go @@ -0,0 +1,328 @@ +package mssql + +import ( + "bytes" + "context" + "encoding/binary" + "fmt" + "math" + "strings" + "testing" + "time" + + "github.com/microsoft/go-mssqldb/msdsn" + "github.com/stretchr/testify/assert" +) + +func variantStream(typeID byte, properties, data []byte) []byte { + stream := binary.LittleEndian.AppendUint32(nil, uint32(2+len(properties)+len(data))) + stream = append(stream, typeID, byte(len(properties))) + stream = append(stream, properties...) + return append(stream, data...) +} + +func TestReadVariantType_RejectsInvalidWidths(t *testing.T) { + fixed := []struct { + typeID byte + width int + }{ + {typeGuid, 16}, {typeBit, 1}, {typeInt1, 1}, {typeInt2, 2}, + {typeInt4, 4}, {typeInt8, 8}, {typeDateTime, 8}, {typeDateTim4, 4}, + {typeFlt4, 4}, {typeFlt8, 8}, {typeMoney4, 4}, {typeMoney, 8}, {typeDateN, 3}, + } + for _, tc := range fixed { + widths := []int{tc.width - 1, tc.width + 1} + if tc.width > 1 { + widths = append(widths, 0) + } + for _, width := range widths { + t.Run(fmt.Sprintf("%02x/%d", tc.typeID, width), func(t *testing.T) { + stream := variantStream(tc.typeID, nil, make([]byte, width)) + checkInvalidVariantBoundary(t, stream) + }) + } + t.Run(fmt.Sprintf("%02x/unexpected property", tc.typeID), func(t *testing.T) { + checkInvalidVariantBoundary(t, variantStream(tc.typeID, []byte{0}, make([]byte, tc.width))) + }) + } + // Exact review repro: a one-byte bigint payload followed by another value. + t.Run("bigint one byte", func(t *testing.T) { + checkInvalidVariantBoundary(t, []byte{3, 0, 0, 0, typeInt8, 0, 0x12}) + }) + for _, typeID := range []byte{typeDecimalN, typeNumericN} { + for _, width := range []int{0, 1, 4, 6, 18, 21} { + t.Run(fmt.Sprintf("%02x/%d", typeID, width), func(t *testing.T) { + checkInvalidVariantBoundary(t, variantStream(typeID, []byte{38, 0}, make([]byte, width))) + }) + } + } + for _, typeID := range []byte{typeTimeN, typeDateTime2N, typeDateTimeOffsetN} { + for _, scale := range []byte{0, 2, 3, 4, 5, 7} { + width := []int{3, 3, 3, 4, 4, 5, 5, 5}[scale] + if typeID == typeDateTime2N { + width += 3 + } else if typeID == typeDateTimeOffsetN { + width += 5 + } + for _, length := range []int{width - 1, width + 1} { + t.Run(fmt.Sprintf("%02x/scale%d/%d", typeID, scale, length), func(t *testing.T) { + checkInvalidVariantBoundary(t, variantStream(typeID, []byte{scale}, make([]byte, length))) + }) + } + } + for _, scale := range []byte{8, 255} { + t.Run(fmt.Sprintf("%02x/scale%d", typeID, scale), func(t *testing.T) { + checkInvalidVariantBoundary(t, variantStream(typeID, []byte{scale}, make([]byte, 10))) + }) + } + } +} + +func checkInvalidVariantBoundary(t *testing.T, stream []byte) { + t.Helper() + sentinel := []byte{0xde, 0xad, 0xbe, 0xef, 1, 2, 3, 4, 0xa5} + r := bufFromBytes(append(append([]byte{}, stream...), sentinel...)) + defer r.bufClose() + err := recoverErr(func() { + readVariantTypeWithEncoding(&typeInfo{}, r, nil, msdsn.EncodeParameters{}) + }) + if r.rpos > len(stream) { + t.Fatalf("sql_variant consumed %d bytes past its boundary", r.rpos-len(stream)) + } + assertStreamError(t, err) + assert.Equal(t, sentinel, r.rbuf[len(stream):r.rsize]) + conn := &Conn{connectionGood: true} + assert.Equal(t, err, conn.checkBadConn(context.Background(), err, false)) + assert.False(t, conn.connectionGood) +} + +func TestReadVariantType_PropertyWidths(t *testing.T) { + cases := []struct { + typeID byte + properties []byte + data []byte + }{ + {typeInt8, nil, make([]byte, 8)}, + {typeTimeN, []byte{7}, make([]byte, 5)}, + {typeDateTime2N, []byte{7}, make([]byte, 8)}, + {typeDateTimeOffsetN, []byte{7}, make([]byte, 10)}, + {typeDecimalN, []byte{9, 0}, []byte{1, 0, 0, 0, 0}}, + {typeNumericN, []byte{9, 0}, []byte{1, 0, 0, 0, 0}}, + {typeBigVarBin, []byte{8, 0}, []byte{1, 2}}, + {typeBigBinary, []byte{2, 0}, []byte{1, 2}}, + {typeBigVarChar, []byte{9, 4, 0, 0, 0, 8, 0}, []byte("hi")}, + {typeBigChar, []byte{9, 4, 0, 0, 0, 2, 0}, []byte("hi")}, + {typeNVarChar, []byte{9, 4, 0, 0, 0, 8, 0}, ucs2("hi")}, + {typeNChar, []byte{9, 4, 0, 0, 0, 4, 0}, ucs2("hi")}, + } + for _, tc := range cases { + for _, size := range []int{0, len(tc.properties) + 1} { + if size == len(tc.properties) { + continue + } + t.Run(fmt.Sprintf("%02x/%d", tc.typeID, size), func(t *testing.T) { + props := make([]byte, size) + copy(props, tc.properties) + checkInvalidVariantBoundary(t, variantStream(tc.typeID, props, tc.data)) + }) + } + } +} + +func TestReadVariantType_ValuesAndPacketBoundaries(t *testing.T) { + guid := []byte{0xff, 0x19, 0x96, 0x6f, 0x86, 0x8b, 0x11, 0xd0, 0xb4, 0x2d, 0, 0xc0, 0x4f, 0xc9, 0x64, 0xff} + type variantCase struct { + typeID byte + properties []byte + data []byte + want interface{} + } + cases := []variantCase{ + {typeGuid, nil, guid, guid}, + {typeBit, nil, []byte{1}, true}, + {typeInt1, nil, []byte{42}, int64(42)}, + {typeInt2, nil, []byte{0xfe, 0xff}, int64(-2)}, + {typeInt4, nil, []byte{0xfd, 0xff, 0xff, 0xff}, int64(-3)}, + {typeInt8, nil, []byte{0xfc, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, int64(-4)}, + {typeDateTime, nil, make([]byte, 8), time.Date(1900, 1, 1, 0, 0, 0, 0, time.UTC)}, + {typeDateTim4, nil, make([]byte, 4), time.Date(1900, 1, 1, 0, 0, 0, 0, time.UTC)}, + {typeFlt4, nil, binary.LittleEndian.AppendUint32(nil, math.Float32bits(0.125)), float64(0.125)}, + {typeFlt8, nil, binary.LittleEndian.AppendUint64(nil, math.Float64bits(0.125)), float64(0.125)}, + {typeMoney4, nil, binary.LittleEndian.AppendUint32(nil, 12345), []byte("1.2345")}, + {typeMoney, nil, []byte{0, 0, 0, 0, 0x39, 0x30, 0, 0}, []byte("1.2345")}, + {typeDateN, nil, []byte{0, 0, 0}, time.Date(1, 1, 1, 0, 0, 0, 0, time.UTC)}, + {typeBigVarBin, []byte{8, 0}, []byte{1, 2}, []byte{1, 2}}, + {typeBigBinary, []byte{2, 0}, []byte{1, 2}, []byte{1, 2}}, + {typeBigVarBin, []byte{8, 0}, []byte{}, []byte{}}, + {typeBigVarChar, []byte{9, 4, 0, 0, 0, 8, 0}, []byte("hi"), "hi"}, + {typeBigChar, []byte{9, 4, 0, 0, 0, 2, 0}, []byte("hi"), "hi"}, + {typeNVarChar, []byte{9, 4, 0, 0, 0, 8, 0}, ucs2("hi"), "hi"}, + {typeNChar, []byte{9, 4, 0, 0, 0, 4, 0}, ucs2("hi"), "hi"}, + {typeBigVarChar, []byte{9, 4, 0, 0, 0, 8, 0}, []byte{}, ""}, + {typeNVarChar, []byte{9, 4, 0, 0, 0, 8, 0}, []byte{}, ""}, + } + for _, typeID := range []byte{typeDecimalN, typeNumericN} { + for _, width := range []int{5, 9, 13, 17} { + data := make([]byte, width) + data[1] = 5 + cases = append(cases, variantCase{typeID, []byte{38, 1}, data, []byte("-0.5")}) + } + } + for scale, width := range []int{3, 3, 3, 4, 4, 5, 5, 5} { + cases = append(cases, + variantCase{typeTimeN, []byte{byte(scale)}, make([]byte, width), time.Date(1, 1, 1, 0, 0, 0, 0, time.UTC)}, + variantCase{typeDateTime2N, []byte{byte(scale)}, make([]byte, width+3), time.Date(1, 1, 1, 0, 0, 0, 0, time.UTC)}, + variantCase{typeDateTimeOffsetN, []byte{byte(scale)}, make([]byte, width+5), time.Date(1, 1, 1, 0, 0, 0, 0, time.FixedZone("", 0))}, + ) + } + for i, tc := range cases { + for _, chunk := range []int{0, 1, 3} { + t.Run(fmt.Sprintf("%02x/%d/%d", tc.typeID, i, chunk), func(t *testing.T) { + value := variantStream(tc.typeID, tc.properties, tc.data) + stream := append(bytes.Repeat(value, 2), 0xa5) + packets, ok := frameReplyPackets(stream, chunk) + if !ok { + t.Fatal("failed to frame variant") + } + if chunk > 0 { + packetCount := (len(stream) + chunk - 1) / chunk + if len(packets) != len(stream)+packetCount*headerSize { + t.Fatal("variant fixture did not use the requested packet boundaries") + } + } + sess := newFuzzSession(packets) + defer sess.buf.bufClose() + if _, err := sess.buf.BeginRead(); err != nil { + t.Fatal(err) + } + ti := typeInfo{TypeId: typeVariant} + for attempt := 0; attempt < 2; attempt++ { + assert.Equal(t, tc.want, readVariantTypeWithEncoding(&ti, sess.buf, nil, msdsn.EncodeParameters{Timezone: time.UTC})) + } + assert.Equal(t, byte(0xa5), sess.buf.byte()) + }) + } + } +} + +func TestReadVariantType_HeaderAndTruncation(t *testing.T) { + for _, stream := range [][]byte{ + {1, 0, 0, 0, typeInt8}, + {0xff, 0xff, 0xff, 0xff}, + {0, 0, 0, 0x80}, + variantStream(0xff, nil, []byte{0}), + } { + t.Run(fmt.Sprintf("%x", stream), func(t *testing.T) { + checkInvalidVariantBoundary(t, stream) + }) + } + value := variantStream(typeInt8, nil, make([]byte, 8)) + for end := 0; end < len(value); end++ { + t.Run(fmt.Sprintf("truncated/%d", end), func(t *testing.T) { + r := bufFromBytes(value[:end]) + defer r.bufClose() + err := recoverErr(func() { + readVariantTypeWithEncoding(&typeInfo{}, r, nil, msdsn.EncodeParameters{}) + }) + assertStreamError(t, err) + }) + } + r := bufFromBytes([]byte{0, 0, 0, 0, 0xa5}) + defer r.bufClose() + assert.Nil(t, readVariantTypeWithEncoding(&typeInfo{}, r, nil, msdsn.EncodeParameters{})) + assert.Equal(t, byte(0xa5), r.byte()) +} + +func variantResponse(value []byte) []byte { + stream := []byte{ + byte(tokenColMetadata), 2, 0, + 0, 0, 0, 0, 0, 0, typeVariant, + } + stream = binary.LittleEndian.AppendUint32(stream, _MAX_VARIANT_LEN) + stream = append(stream, 0, 0, 0, 0, 0, 0, 0, typeInt4, 0, byte(tokenRow)) + stream = append(stream, value...) + stream = binary.LittleEndian.AppendUint32(stream, 123) + return append(stream, doneToken(tokenDone, 0)...) +} + +func TestProcessSingleResponse_VariantBoundary(t *testing.T) { + valid := variantStream(typeInt8, nil, binary.LittleEndian.AppendUint64(nil, 42)) + for _, chunk := range []int{0, 1, 3} { + tokens, _, sawError, framed := drainSingleResponse(variantResponse(valid), chunk, true) + if !framed || sawError { + t.Fatalf("valid variant response failed: %v", tokens) + } + assert.Contains(t, tokens, "row[42 123]") + + invalid := variantStream(typeInt8, nil, []byte{0x12}) + tokens, _, sawError, framed = drainSingleResponse(variantResponse(invalid), chunk, true) + if !framed || !sawError { + t.Fatal("malformed variant response did not fail") + } + assert.Contains(t, tokens, "error:mssql.StreamError") + for _, tok := range tokens { + if strings.HasPrefix(tok, "row") { + t.Fatalf("malformed variant emitted a row before failing: %s", tok) + } + } + } +} + +func TestReadVariantType_ReturnValueBoundary(t *testing.T) { + header := []byte{0, 0, 0, 1, 0, 0, 0, 0, 0, 0, typeVariant} + header = binary.LittleEndian.AppendUint32(header, _MAX_VARIANT_LEN) + for _, width := range []int{1, 8} { + t.Run(fmt.Sprint(width), func(t *testing.T) { + value := variantStream(typeInt8, nil, make([]byte, width)) + stream := append(append([]byte{}, header...), value...) + boundary := len(stream) + stream = append(stream, 0xa5, 1, 2, 3, 4, 5, 6, 7) + r := bufFromBytes(stream) + defer r.bufClose() + err := recoverErr(func() { + got := parseReturnValue(r, &tdsSession{}) + assert.Equal(t, int64(0), got.Value) + }) + if width == 1 { + assertStreamError(t, err) + assert.LessOrEqual(t, r.rpos, boundary) + } else { + assert.NoError(t, err) + assert.Equal(t, boundary, r.rpos) + assert.Equal(t, byte(0xa5), r.byte()) + } + }) + } +} + +func TestReadVariantType_NbcRowBoundary(t *testing.T) { + for _, width := range []int{1, 8} { + t.Run(fmt.Sprint(width), func(t *testing.T) { + value := variantStream(typeInt8, nil, make([]byte, width)) + metadata := bufFromBytes(variantResponse(value)[1:]) + defer metadata.bufClose() + columns := parseColMetadata72(metadata, &tdsSession{}) + stream := append([]byte{0}, value...) + boundary := len(stream) + stream = binary.LittleEndian.AppendUint32(stream, 123) + stream = append(stream, 0xa5, 1, 2, 3, 4, 5, 6, 7) + r := bufFromBytes(stream) + defer r.bufClose() + row := make([]interface{}, 2) + err := recoverErr(func() { + if err := parseNbcRow(context.Background(), r, &tdsSession{}, columns, row); err != nil { + t.Fatal(err) + } + }) + if width == 1 { + assertStreamError(t, err) + assert.LessOrEqual(t, r.rpos, boundary) + assert.Equal(t, []interface{}{nil, nil}, row) + } else { + assert.NoError(t, err) + assert.Equal(t, []interface{}{int64(0), int64(123)}, row) + assert.Equal(t, byte(0xa5), r.byte()) + } + }) + } +} From 2e0b392cbe02e8791c99c425111aaa858b1f93d7 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Tue, 22 Sep 2026 20:26:34 -0700 Subject: [PATCH 37/38] refactor: use tagged switches for variant widths Address QF1003 in the new date/time variant width checks and their regression fixtures without changing behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- types.go | 5 +++-- types_variant_test.go | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/types.go b/types.go index 7f2999f7d..b70567eda 100644 --- a/types.go +++ b/types.go @@ -781,9 +781,10 @@ func readVariantTypeWithEncoding(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, badStreamPanic(fmt.Errorf("sql_variant type 0x%x has invalid scale %d", vartype, scale)) } wantData := int32(calcTimeSize(int(scale))) - if vartype == typeDateTime2N { + switch vartype { + case typeDateTime2N: wantData += 3 - } else if vartype == typeDateTimeOffsetN { + case typeDateTimeOffsetN: wantData += 5 } checkLengths(1, wantData) diff --git a/types_variant_test.go b/types_variant_test.go index bc82aec73..9a9920006 100644 --- a/types_variant_test.go +++ b/types_variant_test.go @@ -59,9 +59,10 @@ func TestReadVariantType_RejectsInvalidWidths(t *testing.T) { for _, typeID := range []byte{typeTimeN, typeDateTime2N, typeDateTimeOffsetN} { for _, scale := range []byte{0, 2, 3, 4, 5, 7} { width := []int{3, 3, 3, 4, 4, 5, 5, 5}[scale] - if typeID == typeDateTime2N { + switch typeID { + case typeDateTime2N: width += 3 - } else if typeID == typeDateTimeOffsetN { + case typeDateTimeOffsetN: width += 5 } for _, length := range []int{width - 1, width + 1} { From c1af3491120e1f9ab83ddcb18cb40fcebd4aa4a7 Mon Sep 17 00:00:00 2001 From: "Saurabh Singh (SQL Drivers)" Date: Tue, 22 Sep 2026 21:24:45 -0700 Subject: [PATCH 38/38] fix: reject odd Unicode variant lengths as stream errors Reject incomplete UTF-16 code units before reading variant properties or payloads so malformed results retire the connection. Cover both Unicode types, empty and even-length values, consecutive reads, fragmented responses, and permanent fuzz seeds. Fixes #420 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8081545c-165e-47b4-b155-797228e2ee67 --- token_fuzz_test.go | 2 ++ types.go | 3 ++ types_variant_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/token_fuzz_test.go b/token_fuzz_test.go index bb16a3129..439a819c9 100644 --- a/token_fuzz_test.go +++ b/token_fuzz_test.go @@ -629,6 +629,8 @@ func FuzzProcessSingleResponse(f *testing.F) { variantStream(typeInt8, nil, []byte{0x12}), variantStream(typeTimeN, nil, []byte{7, 0, 0, 0, 0, 0}), variantStream(typeDecimalN, []byte{38, 0}, make([]byte, 21)), + variantStream(typeNVarChar, []byte{9, 4, 0, 0, 0, 8, 0}, []byte{'a'}), + variantStream(typeNChar, []byte{9, 4, 0, 0, 0, 8, 0}, []byte{'a'}), variantStream(typeInt8, nil, binary.LittleEndian.AppendUint64(nil, 42)), } { f.Add(variantResponse(value), uint16(0)) diff --git a/types.go b/types.go index b70567eda..f94a6017a 100644 --- a/types.go +++ b/types.go @@ -824,6 +824,9 @@ func readVariantTypeWithEncoding(ti *typeInfo, r *tdsBuffer, c *cryptoMetadata, return decodeChar(col, buf) case typeNVarChar, typeNChar: checkLengths(7, datalen) + if datalen%2 != 0 { + badStreamPanic(fmt.Errorf("sql_variant type 0x%x has odd UTF-16 data length %d", vartype, datalen)) + } _ = readCollation(r) r.uint16() // max length, ignoring buf := make([]byte, datalen) diff --git a/types_variant_test.go b/types_variant_test.go index 9a9920006..1817410fe 100644 --- a/types_variant_test.go +++ b/types_variant_test.go @@ -130,6 +130,70 @@ func TestReadVariantType_PropertyWidths(t *testing.T) { } } +func TestReadVariantType_UnicodeLengths(t *testing.T) { + properties := []byte{9, 4, 0, 0, 0, 8, 0} + for _, typeID := range []byte{typeNVarChar, typeNChar} { + for length := 0; length <= 6; length++ { + t.Run(fmt.Sprintf("%02x/%d", typeID, length), func(t *testing.T) { + payload := ucs2("abc")[:length] + value := variantStream(typeID, properties, payload) + r := bufFromBytes(append(bytes.Repeat(value, 2), 0xa5)) + defer r.bufClose() + ti := typeInfo{TypeId: typeVariant} + conn := &Conn{connectionGood: true} + for attempt := 0; attempt < 2; attempt++ { + start := r.rpos + var got interface{} + err := recoverErr(func() { + got = readVariantTypeWithEncoding(&ti, r, nil, msdsn.EncodeParameters{}) + }) + if length%2 != 0 { + assertStreamError(t, err) + assert.Contains(t, err.Error(), "UTF-16") + assert.Equal(t, start+6, r.rpos, "reject before reading properties or payload") + assert.Equal(t, err, conn.checkBadConn(context.Background(), err, false)) + assert.False(t, conn.connectionGood) + return + } + assert.NoError(t, err) + assert.Equal(t, "abc"[:length/2], got) + assert.Equal(t, start+len(value), r.rpos) + assert.Nil(t, conn.checkBadConn(context.Background(), err, false)) + assert.True(t, conn.connectionGood) + } + assert.Equal(t, byte(0xa5), r.byte()) + }) + } + } +} + +func TestProcessSingleResponse_VariantUnicodeLengths(t *testing.T) { + properties := []byte{9, 4, 0, 0, 0, 8, 0} + for _, typeID := range []byte{typeNVarChar, typeNChar} { + for _, chunk := range []int{0, 1, 3} { + t.Run(fmt.Sprintf("%02x/%d", typeID, chunk), func(t *testing.T) { + value := variantStream(typeID, properties, []byte{'a'}) + tokens, _, sawError, framed := drainSingleResponse(variantResponse(value), chunk, true) + if !framed || !sawError { + t.Fatal("expected malformed Unicode variant to fail") + } + assert.Contains(t, tokens, "error:mssql.StreamError") + for _, tok := range tokens { + if strings.HasPrefix(tok, "row") { + t.Fatalf("malformed Unicode variant emitted a row: %s", tok) + } + } + value = variantStream(typeID, properties, ucs2("hi")) + tokens, _, sawError, framed = drainSingleResponse(variantResponse(value), chunk, true) + if !framed || sawError { + t.Fatalf("valid Unicode variant failed: %v", tokens) + } + assert.Contains(t, tokens, "row[hi 123]") + }) + } + } +} + func TestReadVariantType_ValuesAndPacketBoundaries(t *testing.T) { guid := []byte{0xff, 0x19, 0x96, 0x6f, 0x86, 0x8b, 0x11, 0xd0, 0xb4, 0x2d, 0, 0xc0, 0x4f, 0xc9, 0x64, 0xff} type variantCase struct {