Skip to content

Security: strix-tool/strix-inspector

SECURITY.md

Strix Inspector (Windows) — Security Analysis & Hardening

Scope: the native StrixInspector.exe (C#/.NET Framework) — detection, benchmarks, HTML report, console/JSON output. Result: 7 hardening changes + 16 security regression tests (tests/SecurityTests.cs, all passing).

Threat model — why the attack surface is small

Strix Inspector is a local, read-only diagnostics tool. It has:

  • No network — nothing is sent or received; the report footer even says so.
  • No elevation — it runs asInvoker and never requests admin (disk sizes need admin but degrade gracefully; nothing else does).
  • No untrusted input path — command-line args come from the user running their own tool. The only externally-influenced bytes are the STORAGE_DEVICE_DESCRIPTOR a storage driver returns, and the hardware strings (disk model/serial, GPU name, BIOS fields) that end up in the report.
  • No writes to sensitive locations — it writes one HTML file where the user asks, and a benchmark temp file with a random name.
  • No eval, no dynamic code, no shell-out, no WMI/PowerShell child process.

So there are no "gaping holes" to close. The work below is genuine defence-in-depth and robustness on the two surfaces that do touch externally-influenced data (driver buffers and hardware strings in HTML), plus standard Windows-native hygiene.

Findings & fixes

1. Driver-returned buffer parsing — bounds hardening — Detect.cs

GetDisks parses a STORAGE_DEVICE_DESCRIPTOR whose header contains offsets (vendor/product/serial) that a buggy or malicious storage driver controls. The original code read strings bounded only by the 1 KiB buffer length.

Fix: the parse is now a pure, isolated function ParseDeviceDescriptor(byte[] buf, int ret) that:

  • rejects responses shorter than a valid header (ret < 36null);
  • clamps every string read to min(ret, buffer length, descriptor.Size) — the driver can never steer a read past the data it actually returned;
  • validates the length IOCTL returned ≥ 8 bytes before reading the size.

AsciiAt takes an explicit valid bound and scans only within it. Because the function is P/Invoke-free it is unit-tested with hostile offsets (-1, int.MinValue, int.MaxValue, past-buffer) — none crash or read out of bounds (proven in tests/SecurityTests.cs).

2. HTML report injection (XSS) — Report.cs

Hardware strings (disk model/serial, GPU name, BIOS vendor/version) can contain markup. Every value written to the report goes through WebUtility.HtmlEncode. Regression tests inject <script>, an attribute-breakout "><img onerror=...>, and <b> payloads into disk/GPU fields and assert the output is escaped.

3. Explicit least-privilege manifest — src/app.manifest

The exe now embeds a manifest declaring requestedExecutionLevel level="asInvoker". This makes least-privilege explicit and stops Windows' installer-detection heuristic from ever auto-prompting for elevation. It also sets longPathAware, UTF-8 activeCodePage, and Win10/11 supportedOS so version APIs report the true OS.

4. Handle-leak & device-enumeration robustness — Detect.cs

Physical-drive handles are released in a finally for every path. The enumeration loop no longer stops at the first missing drive (drive numbers can have gaps after a removal); it skips gaps and stops after 8 consecutive misses, so all disks are found without scanning forever.

5. Top-level exception guard — Program.cs

Main wraps execution in try/catch and prints a one-line message instead of letting .NET dump a stack trace (which can leak internal file paths) to the console. Returns a distinct exit code.

6. Benchmark divide-by-zero — Bench.cs

All four throughput calculations divide elapsed time with a 1e-6 floor, so a sub-microsecond measurement can never produce Infinity/NaN in the output.

7. Removed static mutable state — Detect.cs

A static field previously carried the "run as admin" note out of GetDisks (not thread-safe, persists across calls). Notes now flow through a parameter.

Windows-native hygiene (already correct — verified, not changed)

  • No DLL-planting vector. Every [DllImport] targets a KnownDLL (kernel32, ntdll, user32), which Windows always loads from System32 — a DLL dropped in the app folder cannot hijack them. There are no external managed dependencies either (BCL-only), so no managed-assembly planting.
  • Read-only physical-disk access. Disks are opened with 0 or GENERIC_READ and only query IOCTLs are issued — never a write. Property queries work without admin; the size IOCTL is the only admin-gated call and fails soft.
  • Registry is opened read-only (OpenSubKey) and disposed; values are type-checked (as string / TryParse).
  • Benchmark temp file uses a Guid-random name in the per-user temp dir and is deleted in finally — no predictable-name/symlink foothold.
  • Marshalling uses fixed StructLayout with correct CharSet; the \\.\PhysicalDriveN path is built from an integer loop variable (no injection).

The one thing outside code: signing

The exe is unsigned, so Windows shows a SmartScreen "unknown publisher" prompt on first run and it is more antivirus-false-positive-prone. That is a distribution property, not a code vulnerability; the fix is code signing (Build.ps1 -Sign -Thumbprint …) — see the disk-cleaner packaging notes for the current 2026 certificate guidance.

Tests

mcs -out:sectest.exe tests/SecurityTests.cs src/Model.cs src/Native.cs src/Detect.cs src/Bench.cs src/Report.cs
mono sectest.exe          # 16 passed — ALL SECURITY TESTS PASSED

Covered: hostile descriptor offsets (no OOB/throw), truncated/null buffers, offset clamping to driver-reported length, well-formed parse correctness, and HTML/XSS escaping of hardware strings.

Enrichment parsers (SMBIOS memory + processor info)

Two new byte-parsers read externally-supplied firmware data (the SMBIOS table via GetSystemFirmwareTable, and GetLogicalProcessorInformation records). Both are pure functions (ParseSmbiosMemory, ParseProcInfo) that are bounds- checked and unit-tested (tests/ParserTests.cs, all passing) against: truncated tables, malformed structure lengths, missing end markers, and arch-specific record layouts. A malformed table can lose sync but never reads out of range or throws. The GPU VRAM/driver read is registry-only (read-only, type-checked). All new fields flow into the report through the same HtmlEncode escaping.

Re-check pass (after adding the dashboard UI)

The UI upgrade added a browser dashboard, a theme toggle (JS), and new displayed fields (disk serials, GPU names). Re-reviewed for the surfaces that change:

  • Escaping still holds on the new template. Every hardware value in the dashboard is emitted via WebUtility.HtmlEncode into element text content — never into an HTML attribute or a JS string. Verified that an injected "><img onerror=...> serial is rendered as inert escaped text (&quot;&gt;&lt;img …&gt;): no live tag, no attribute breakout. The theme toggle script is static and reads no hardware data.
  • Report is written to a user-writable path. The double-click dashboard now writes to %TEMP% rather than next to the exe — so it works even when the app is installed under Program Files without admin (previously that write would throw). Fixed.
  • Browser launch is safe. OpenInBrowser runs Process.Start with UseShellExecute=true only on a local file path this program just wrote — no user/network string reaches it.
  • Test assertion corrected. One XSS test checked for a substring (onerror=alert) that legitimately survives as escaped text once serials are displayed; it now asserts the real property (no live <img / no "> breakout). All 16 pass.

There aren't any published security advisories