registry/hive: reject malformed cell sizes instead of panicking

readCell trusted the cell's size header without bounds-checking the
positive side, so any cell whose raw size field was 0 or a negative
value with absolute magnitude smaller than the 4-byte cell header
produced an inverted slice expression (h.data[pos+4 : pos+size] with
size < 4) and crashed the process. Reported as a runtime panic
`slice bounds out of range [5052:5048]` in secretsdump's cached
domain logon parse path.

Treat raw size >= 0 as free/invalid (return error) and require the
allocated size to be at least 4 bytes so the data slice is never
inverted. Regression test synthesizes a minimal hive with each of
the previously-panicking inputs.

Removes the now-resolved entry from KNOWN_ISSUES.md and renumbers
the remaining sections.
This commit is contained in:
psycep
2026-04-23 15:23:59 -05:00
parent 4b90cdefd0
commit 2ba52b5116
3 changed files with 108 additions and 26 deletions
+7 -19
View File
@@ -26,19 +26,7 @@ When reporting, please include:
---
## 2. Standalone secretsdump: Panic in Cached Credentials Parser
**Symptom:** `panic: runtime error: slice bounds out of range [5052:5048]` in `pkg/registry/hive.go:82` when parsing the SECURITY hive's cached domain logon entries.
**Details:** SAM hashes and LSA secrets dump correctly, but parsing `NL$` cached credential entries causes an out-of-bounds slice access in the registry hive cell reader. Occurs after successfully dumping several cached entries.
**Workaround:** SAM hashes and LSA secrets are dumped before the panic occurs, so those results are usable. The panic only affects cached domain logon credentials.
**Status:** Bug in `pkg/registry/hive.go` `readCell()` — needs bounds checking fix.
---
## 3. tschexec / enum-local-admins: RPC Access Denied
## 2. tschexec / enum-local-admins: RPC Access Denied
**Symptom:** `RPC Fault 0x05 (ACCESS_DENIED)` when running `tschexec` or `enumlocaladmins` via relay.
@@ -52,7 +40,7 @@ When reporting, please include:
---
## 4. SMB Relay: Intermittent PIPE_NOT_AVAILABLE (0xc00000ac)
## 3. SMB Relay: Intermittent PIPE_NOT_AVAILABLE (0xc00000ac)
**Symptom:** `create failed: status=0xc00000ac` when opening the `winreg` named pipe on the relay target.
@@ -64,7 +52,7 @@ When reporting, please include:
---
## 5. Shadow Credentials: Certificate Generation Not Implemented
## 4. Shadow Credentials: Certificate Generation Not Implemented
**Symptom:** `-attack shadowcreds` reads existing `msDS-KeyCredentialLink` values but cannot write new shadow credentials.
@@ -76,7 +64,7 @@ When reporting, please include:
---
## 6. LDAP Relay: Plain LDAP (Port 389) Post-Auth Signing Failure
## 5. LDAP Relay: Plain LDAP (Port 389) Post-Auth Signing Failure
**Symptom:** LDAP relay to port 389 authenticates successfully but subsequent LDAP operations fail with signing errors on patched DCs.
@@ -88,7 +76,7 @@ When reporting, please include:
---
## 7. SMB→LDAPS Relay Fails on Patched DCs
## 6. SMB→LDAPS Relay Fails on Patched DCs
**Symptom:** Relay from SMB capture to LDAPS target fails with MIC validation errors.
@@ -102,7 +90,7 @@ When reporting, please include:
---
## 8. UDP Features Disabled Under `-proxy`
## 7. UDP Features Disabled Under `-proxy`
**Symptom:** Tools that depend on UDP fail with `UDP disabled under -proxy; the underlying feature cannot be tunneled` when `-proxy` (or `ALL_PROXY`) is set.
@@ -122,7 +110,7 @@ When reporting, please include:
---
## 9. Remaining Gaps (Low Priority)
## 8. Remaining Gaps (Low Priority)
These Impacket features are not yet implemented due to infrastructure requirements:
+13 -7
View File
@@ -75,20 +75,26 @@ func (h *Hive) cellOffset(offset int32) int {
return int(offset) + 4096
}
// readCell reads a cell at the given offset and returns its data
// readCell reads a cell at the given offset and returns its data (the bytes
// after the 4-byte cell header). A valid allocated cell has a negative size
// field; a positive size marks a free cell, and zero is malformed.
func (h *Hive) readCell(offset int32) ([]byte, error) {
pos := h.cellOffset(offset)
if pos < 4096 || pos >= len(h.data)-4 {
return nil, fmt.Errorf("invalid cell offset: %d", offset)
}
// Cell size is first 4 bytes (negative = allocated, positive = free)
size := int32(binary.LittleEndian.Uint32(h.data[pos : pos+4]))
if size > 0 {
return nil, fmt.Errorf("cell is free at offset %d", offset)
rawSize := int32(binary.LittleEndian.Uint32(h.data[pos : pos+4]))
if rawSize >= 0 {
// Positive = free cell, zero = malformed. Either way, no data to read.
return nil, fmt.Errorf("cell at offset %d is free or zero-sized (raw size %d)", offset, rawSize)
}
size := -rawSize
if size < 4 {
// Minimum cell is the 4-byte header; anything smaller would produce
// an inverted slice (panic in the original code).
return nil, fmt.Errorf("cell at offset %d has invalid size %d (min 4)", offset, size)
}
size = -size
if pos+int(size) > len(h.data) {
return nil, fmt.Errorf("cell extends beyond hive: %d + %d > %d", pos, size, len(h.data))
}
+88
View File
@@ -0,0 +1,88 @@
// 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
package registry
import (
"encoding/binary"
"strings"
"testing"
)
// synthHive builds the minimum bytes Open() will accept: a valid header with
// signature, root offset, and zeroed first hbin. Tests use this to construct
// hives with specific cell bytes without needing a real registry file.
func synthHive(t *testing.T, cellsAt4096 []byte) *Hive {
t.Helper()
// 4096 bytes of header + 4096 bytes of hbin = 8192 minimum.
data := make([]byte, 8192+len(cellsAt4096))
// Header signature 'regf'
binary.LittleEndian.PutUint32(data[4:], regfMagic)
// Root offset at header[36:40] is irrelevant for readCell-only tests.
binary.LittleEndian.PutUint32(data[36:40], 0)
copy(data[4096:], cellsAt4096)
return &Hive{data: data, rootOffset: 0}
}
// TestReadCellRejectsMalformedSizes covers the inputs that used to panic in
// pkg/registry/hive.go: cell headers whose raw size field was 0 or a
// negative value with absolute magnitude smaller than the 4-byte header.
// All of these must now return an error, not crash.
func TestReadCellRejectsMalformedSizes(t *testing.T) {
cases := []struct {
name string
rawSize int32
wantSub string
}{
{"zero size", 0, "free or zero-sized"},
{"positive (free cell)", 64, "free or zero-sized"},
{"negative -1", -1, "invalid size"},
{"negative -3", -3, "invalid size"},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
// Lay the raw cell header at the very start of the first hbin
// (hbin data begins at offset 4096 in the hive, offset 0 from
// the cell-offset perspective).
cell := make([]byte, 8)
binary.LittleEndian.PutUint32(cell, uint32(c.rawSize))
h := synthHive(t, cell)
defer func() {
if r := recover(); r != nil {
t.Fatalf("readCell panicked on %q: %v", c.name, r)
}
}()
_, err := h.readCell(0)
if err == nil {
t.Fatalf("readCell returned nil error for %q", c.name)
}
if !strings.Contains(err.Error(), c.wantSub) {
t.Fatalf("readCell error %q did not contain %q", err.Error(), c.wantSub)
}
})
}
}
// TestReadCellReturnsDataForValidCell confirms the happy path still works:
// a -16 size header should yield a 12-byte slice.
func TestReadCellReturnsDataForValidCell(t *testing.T) {
cell := make([]byte, 16)
var neg int32 = -16
binary.LittleEndian.PutUint32(cell, uint32(neg))
copy(cell[4:], []byte("hello, world"))
h := synthHive(t, cell)
data, err := h.readCell(0)
if err != nil {
t.Fatalf("readCell error: %v", err)
}
if string(data) != "hello, world" {
t.Fatalf("readCell returned %q, want %q", string(data), "hello, world")
}
}