Cover the GetValueData guard added in the previous commit with two
table-driven tests: the rejection path (dataLen ∈ {5, 8, 0xFF,
0x7FFFFFFF} with resident bit set) confirms the guard fires without
panicking and surfaces a helpful error; the happy path (dataLen ∈
{1, 2, 3, 4}) confirms the inline DataOffset bytes are returned
correctly so the guard didn't regress valid hives.
The matching guards in SetValueData (resident + non-resident write
paths) and enumSubKeys (riSig branch) require full NK/VK/sub-list
hive synthesis to exercise end-to-end; they're structurally identical
to the read-path guard and are validated by build/vet plus the lab
regression check on secretsdump. Worth a follow-up to extend the
synth helpers and cover them directly.
Four parser-panic / silent-corruption bugs in pkg/registry/hive.go,
all reachable from attacker-controlled hive bytes:
1. GetValueData resident branch: VKRecord.DataLen lower 31 bits are
read verbatim and used to slice a 4-byte buffer at [:dataLen]. A
DataLen of 0x80000005 panics with "slice bounds out of range".
Found by kajaaz using Zorya (issue #25); fix matches her suggested
one-line bounds check.
2. SetValueData resident branch (structurally identical to #1): the
existing len(newData) == dataLen check doesn't enforce the 4-byte
cap, so a hostile dataLen=5 with a matching 5-byte newData slices
one byte past the DataOffset field into the adjacent cell. Same
guard.
3. SetValueData non-resident branch: vk.DataOffset is attacker-
controlled and the code does copy(h.data[dataPos:dataPos+dataLen],
newData) without ever validating the destination. Hostile offsets
either panic on out-of-bounds or silently scribble over arbitrary
hive bytes (a value of 0xFFFFF000 lands near the regf header). Route
through readCell (which validates the cell header and bounds) and
verify dataLen fits before mutating.
4. enumSubKeys riSig branch: readCell can return a slice shorter than
the 4-byte cell header (minimum-size cell with no usable bytes), so
the immediate subCell[0:2] / subCell[2:4] reads can panic on a
malformed sub-list. The sibling code at line 397/437 already guards
the analogous index; mirror it here.
Fixes#25 plus three structurally similar bugs surfaced while patching.
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.
Jacob Paullus