diff --git a/pkg/dcerpc/drsuapi/getncchanges.go b/pkg/dcerpc/drsuapi/getncchanges.go index 4166e4f..062a9bf 100644 --- a/pkg/dcerpc/drsuapi/getncchanges.go +++ b/pkg/dcerpc/drsuapi/getncchanges.go @@ -323,41 +323,6 @@ func parseGUID(s string) []byte { return result } -func writePartialAttrSet(buf *bytes.Buffer) { - // PARTIAL_ATTR_VECTOR_V1_EXT is a conformant structure in NDR - // rgPartialAttr is [size_is(cAttrs)] - // Per NDR rules: - // 1. Conformance (MaxCount for rgPartialAttr) comes first - // 2. Then structure fields - - // Attributes we want for password dumping - attrs := []uint32{ - DRSUAPI_ATTID_objectSid, - DRSUAPI_ATTID_sAMAccountName, - DRSUAPI_ATTID_unicodePwd, - DRSUAPI_ATTID_ntPwdHistory, - DRSUAPI_ATTID_dBCSPwd, - DRSUAPI_ATTID_lmPwdHistory, - DRSUAPI_ATTID_supplementalCredentials, - DRSUAPI_ATTID_userAccountControl, - DRSUAPI_ATTID_objectGUID, - DRSUAPI_ATTID_pwdLastSet, - } - - // NDR conformance first - binary.Write(buf, binary.LittleEndian, uint32(len(attrs))) // MaxCount for rgPartialAttr - - // Structure fields - binary.Write(buf, binary.LittleEndian, uint32(1)) // dwVersion - binary.Write(buf, binary.LittleEndian, uint32(0)) // dwReserved1 - binary.Write(buf, binary.LittleEndian, uint32(len(attrs))) // cAttrs - - // Array data - for _, attr := range attrs { - binary.Write(buf, binary.LittleEndian, attr) - } -} - func parseGetNCChangesResponse(resp []byte, sessionKey []byte) (*GetNCChangesResult, error) { if len(resp) < 8 { return nil, fmt.Errorf("response too short") diff --git a/pkg/dcerpc/drsuapi/getncchanges_v6.go b/pkg/dcerpc/drsuapi/getncchanges_v6.go index 700b1a2..4ffb255 100644 --- a/pkg/dcerpc/drsuapi/getncchanges_v6.go +++ b/pkg/dcerpc/drsuapi/getncchanges_v6.go @@ -37,11 +37,13 @@ import ( "github.com/mandiant/gopacket/pkg/utf16le" ) -// maxReasonable bounds the per-array element count any V6 parser helper is -// willing to honour from the wire, so a malformed or hostile reply can't push -// us into a giant allocation or a count*size product that overflows int on -// 32-bit builds. Generous enough for any real DC. -const maxReasonable = 10 * 1024 * 1024 +// Each helper below uses d.CheckBounds(count, elemSize, what) before +// allocating or skipping memory whose size is taken from the wire. The +// element sizes are fixed per call site (ATTR fixed part = 12 bytes, +// ATTRVAL fixed part = 8 bytes, REPLENTINFLIST header = 32 bytes, etc.), +// so a malformed reply with a huge embedded count but a small payload +// gets rejected at parse start rather than after a multi-MB speculative +// make of bytes that don't actually exist on the wire. // parseGetNCChangesResponseV6NDR parses the V6/V7/V9 GetNCChanges reply. // V6 is the canonical shape; V7 and V9 add fields after the V6 body which @@ -135,8 +137,7 @@ func skipDSNAMEv2(d *Decoder) { d.Skip(16) // Guid d.Skip(28) // Sid _ = d.ReadUint32() // NameLen - if maxCount > maxReasonable { - d.Fail("ndr: DSNAME StringName count %d exceeds cap %d", maxCount, maxReasonable) + if !d.CheckBounds(maxCount, 2, "DSNAME StringName") { return } d.Skip(int(maxCount) * 2) @@ -162,8 +163,7 @@ func readDSNAMEv2(d *Decoder, obj *ReplicatedObject) { } } nameLen := d.ReadUint32() - if maxCount > maxReasonable { - d.Fail("ndr: DSNAME StringName count %d exceeds cap %d", maxCount, maxReasonable) + if !d.CheckBounds(maxCount, 2, "DSNAME StringName") { return } if maxCount > 0 { @@ -207,8 +207,7 @@ func skipUpToDateVectorV2(d *Decoder) { if version == 2 { cursorSize = 32 } - if maxCount > maxReasonable { - d.Fail("ndr: UPTODATE_VECTOR cursor count %d exceeds cap %d", maxCount, maxReasonable) + if !d.CheckBounds(maxCount, cursorSize, "UPTODATE_VECTOR cursors") { return } d.Skip(int(maxCount) * cursorSize) @@ -224,8 +223,8 @@ func skipUpToDateVectorV2(d *Decoder) { // the wire conformance keeps the cursor aligned for downstream deferreds. func readPrefixTableV2(d *Decoder, prefixTable map[uint32][]byte) { count := d.ReadConformance() // wire authority for entry count - if count > maxReasonable { - d.Fail("ndr: PrefixTableEntry count %d exceeds cap %d", count, maxReasonable) + // Fixed part per PrefixTableEntry: ndx(4) + oidLen(4) + pOid ref(4) = 12. + if !d.CheckBounds(count, 12, "PrefixTableEntry fixed parts") { return } @@ -249,8 +248,7 @@ func readPrefixTableV2(d *Decoder, prefixTable map[uint32][]byte) { continue } oidMaxCount := d.ReadConformance() - if oidMaxCount > maxReasonable { - d.Fail("ndr: PrefixTableEntry.pOid count %d exceeds cap %d", oidMaxCount, maxReasonable) + if !d.CheckBounds(oidMaxCount, 1, "PrefixTableEntry.pOid") { return } oidBytes := d.ReadBytes(int(oidMaxCount)) @@ -289,13 +287,12 @@ func readPrefixTableV2(d *Decoder, prefixTable map[uint32][]byte) { // depends on where the decoder landed after the preceding pParent UUID, which // is 4-aligned rather than 8-aligned. func skipPropertyMetaDataExtVectorV2(d *Decoder) { - maxCount := d.ReadUint32() // hoisted conformance, 4-aligned + maxCount := d.ReadConformance() // hoisted conformance, primitive 4-aligned d.Align(8) // struct alignment before cNumProps _ = d.ReadUint32() // cNumProps d.Align(8) // element alignment const metaDataExtSize = 40 - if maxCount > maxReasonable { - d.Fail("ndr: PROPERTY_META_DATA_EXT count %d exceeds cap %d", maxCount, maxReasonable) + if !d.CheckBounds(maxCount, metaDataExtSize, "PROPERTY_META_DATA_EXT") { return } d.Skip(int(maxCount) * metaDataExtSize) @@ -322,8 +319,11 @@ func readREPLENTINFLISTArrayV2(d *Decoder, sessionKey []byte, prefixTable map[ui ptrMetaData uint32 } - if numObjects > maxReasonable { - d.Fail("ndr: REPLENTINFLIST cNumObjects %d exceeds cap %d", numObjects, maxReasonable) + // Fixed part per REPLENTINFLIST node: pNext(4) + pName(4) + flags(4) + + // attrCount(4) + ptrAttr(4) + isNCPrefix(4) + ptrParentGuid(4) + + // ptrMetaData(4) = 32. A malformed cNumObjects must not let us + // pre-allocate more capacity than the wire can possibly back. + if !d.CheckBounds(numObjects, 32, "REPLENTINFLIST headers") { return nil } @@ -391,8 +391,8 @@ func readATTRBLOCKv2(d *Decoder, obj *ReplicatedObject, sessionKey []byte, prefi _ = prefixTable arrayMax := d.ReadConformance() - if arrayMax > maxReasonable { - d.Fail("ndr: ATTR_ARRAY count %d exceeds cap %d", arrayMax, maxReasonable) + // Fixed part per ATTR: attrTyp(4) + valCount(4) + pAVal ref(4) = 12. + if !d.CheckBounds(arrayMax, 12, "ATTR_ARRAY fixed parts") { return } @@ -412,8 +412,8 @@ func readATTRBLOCKv2(d *Decoder, obj *ReplicatedObject, sessionKey []byte, prefi continue } valMax := d.ReadConformance() - if valMax > maxReasonable { - d.Fail("ndr: ATTRVAL_ARRAY count %d exceeds cap %d", valMax, maxReasonable) + // Fixed part per ATTRVAL: valLen(4) + pVal ref(4) = 8. + if !d.CheckBounds(valMax, 8, "ATTRVAL_ARRAY fixed parts") { return } vals := make([]uint32, valMax) @@ -426,8 +426,7 @@ func readATTRBLOCKv2(d *Decoder, obj *ReplicatedObject, sessionKey []byte, prefi continue } byteMax := d.ReadConformance() - if byteMax > maxReasonable { - d.Fail("ndr: ATTRVAL byte count %d exceeds cap %d", byteMax, maxReasonable) + if !d.CheckBounds(byteMax, 1, "ATTRVAL bytes") { return } valData := d.ReadBytes(int(byteMax)) @@ -439,19 +438,35 @@ func readATTRBLOCKv2(d *Decoder, obj *ReplicatedObject, sessionKey []byte, prefi } } -// firstRDNValue returns the value portion of the first RDN in dn (e.g. -// "Smith, John" from "Smith\, John,OU=Foo"), honoring RFC 4514's -// backslash-escape rule so embedded commas don't truncate the result. The +// firstRDNValue returns the unescaped value portion of the first RDN in +// dn (e.g. "Smith, John" from "Smith\, John,OU=Foo"), honoring RFC 4514's +// backslash-escape rule so embedded commas don't truncate the result and +// the escape character itself is dropped from the returned value. The // caller has already stripped the "CN=" prefix. +// +// Only the single-char form (\) is unescaped, which is what AD +// emits in practice for the SAM-name extraction path. The two-char hex +// form (\HH) is rare here and is intentionally not decoded — the leading +// hex digit is kept literally, matching the previous one-byte skip +// behavior so we don't accidentally hand callers a different RDN value. func firstRDNValue(dn string) string { + var b strings.Builder + b.Grow(len(dn)) for i := 0; i < len(dn); i++ { - if dn[i] == '\\' { - i++ // skip the escaped char (i++ in the loop header advances past it) + c := dn[i] + if c == '\\' { + if i+1 < len(dn) { + b.WriteByte(dn[i+1]) + i++ + } + // A trailing backslash is a malformed escape per RFC 4514; + // drop it silently rather than emitting a literal '\'. continue } - if dn[i] == ',' { - return dn[:i] + if c == ',' { + return b.String() } + b.WriteByte(c) } - return dn + return b.String() } diff --git a/pkg/dcerpc/drsuapi/getncchanges_v6_test.go b/pkg/dcerpc/drsuapi/getncchanges_v6_test.go new file mode 100644 index 0000000..f9f237d --- /dev/null +++ b/pkg/dcerpc/drsuapi/getncchanges_v6_test.go @@ -0,0 +1,105 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package drsuapi + +import "testing" + +func TestFirstRDNValue(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"plain", "Administrator,CN=Users,DC=example,DC=com", "Administrator"}, + {"no comma", "Administrator", "Administrator"}, + {"empty", "", ""}, + {"escaped comma", `Smith\, John,OU=Foo,DC=example`, "Smith, John"}, + {"escaped equals", `O\=ops,OU=Foo`, "O=ops"}, + {"escaped backslash", `back\\slash,OU=Foo`, `back\slash`}, + {"multiple escapes", `a\,b\,c,OU=Foo`, "a,b,c"}, + {"trailing backslash", `oops\`, "oops"}, + {"only escape", `\,`, ","}, + {"escape then end", `x\,`, "x,"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := firstRDNValue(tc.in) + if got != tc.want { + t.Errorf("firstRDNValue(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestCheckBounds(t *testing.T) { + t.Run("fits exactly", func(t *testing.T) { + d := NewDecoder(make([]byte, 64)) + if !d.CheckBounds(8, 8, "exact") { + t.Fatalf("CheckBounds should accept count*size == remaining; err=%v", d.Err()) + } + if d.Err() != nil { + t.Fatalf("unexpected err: %v", d.Err()) + } + }) + + t.Run("rejects overflow of remaining", func(t *testing.T) { + d := NewDecoder(make([]byte, 31)) + if d.CheckBounds(4, 8, "overflow") { + t.Fatal("CheckBounds should reject 4*8=32 against 31 remaining") + } + if d.Err() == nil { + t.Fatal("expected sticky error") + } + }) + + t.Run("uint64 product is safe", func(t *testing.T) { + // count near MaxUint32 * non-trivial elemSize would overflow int on + // 32-bit builds. The check must reject before any int cast. + d := NewDecoder(make([]byte, 100)) + if d.CheckBounds(1<<31, 8, "huge product") { + t.Fatal("CheckBounds should reject pathologically large product") + } + }) + + t.Run("zero count always fits", func(t *testing.T) { + d := NewDecoder(nil) + if !d.CheckBounds(0, 32, "empty") { + t.Fatalf("CheckBounds(0, 32) on empty buffer should pass; err=%v", d.Err()) + } + }) + + t.Run("prior error is sticky", func(t *testing.T) { + d := NewDecoder(make([]byte, 1024)) + d.Fail("seeded failure") + if d.CheckBounds(1, 1, "post-failure") { + t.Fatal("CheckBounds must return false when decoder already failed") + } + // And the original error must be preserved. + if got := d.Err().Error(); got != "seeded failure" { + t.Fatalf("original error overwritten: %v", got) + } + }) + + t.Run("subsequent reads are no-ops after failure", func(t *testing.T) { + d := NewDecoder(make([]byte, 4)) + if d.CheckBounds(10, 8, "trigger") { + t.Fatal("expected failure") + } + // Decoder must now zero-return on reads. + if v := d.ReadUint32(); v != 0 { + t.Fatalf("expected 0 after failure, got %d", v) + } + }) +} diff --git a/pkg/dcerpc/drsuapi/ndr.go b/pkg/dcerpc/drsuapi/ndr.go index ac9da1a..b15a5d6 100644 --- a/pkg/dcerpc/drsuapi/ndr.go +++ b/pkg/dcerpc/drsuapi/ndr.go @@ -208,6 +208,27 @@ func (d *Decoder) ReadPointer() uint32 { return d.ReadUint32() } // 4-byte aligned). func (d *Decoder) ReadConformance() uint32 { return d.ReadUint32() } +// CheckBounds validates that count elements of elemSize bytes can fit in +// the remaining wire buffer. If not, it Fails the decoder (so subsequent +// reads return zero values) and returns false. Use this before any +// allocation or Skip whose size is driven by a wire-supplied count, so a +// malformed reply with a tiny payload and a huge embedded count can't +// trigger a multi-MB speculative make for bytes that don't actually exist +// on the wire. The product is computed in uint64 so it stays safe on +// 32-bit builds where int(count)*elemSize could otherwise overflow. +func (d *Decoder) CheckBounds(count uint32, elemSize int, what string) bool { + if d.err != nil { + return false + } + need := uint64(count) * uint64(elemSize) + if need > uint64(d.Remaining()) { + d.Fail("ndr: %s: need %d bytes (%d * %d) but only %d remain", + what, need, count, elemSize, d.Remaining()) + return false + } + return true +} + // ReadConformantVaryingHeader reads the MaxCount + Offset + ActualCount // prefix of a conformant-varying array. Returns (maxCount, offset, // actualCount). Most callers ignore offset (always 0) and use actualCount