From 852aec5b7f8d41d287e3a5adf3088c2566a65173 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Wed, 2 Sep 2026 16:13:01 -0400 Subject: [PATCH] Verify the launch-path hardening on Windows, and fix what that found The previous commit said the link-resolving change had never met a real filesystem. Run in the win11 VM against real fixtures, it turned out to reject EVERY legitimate launch path. ResolveDirectoryChain walked up looking for a junction and eventually asked whether "C:\" was a link. That throws DirectoryNotFoundException, and the caller treats a resolve failure as a rejection, so the first legitimate case came back "cannot resolve the real path: Could not find a part of the path 'C:\'". The agent would have failed closed on everything - an outage wearing a security fix's clothes. It compiled with zero warnings and read correctly. Stop at the volume root, and never let "cannot tell whether this is a link" turn into a rejection: TryResolveLink returns null on any failure, and the assembled path is re-checked against the roots regardless, so an unresolvable component still cannot widen what is accepted. One expectation in the probe was mine being wrong rather than the code. 8.3 short names used to be rejected because the lexical prefix check could not match C:\PROGRA~1 against C:\Program Files - fail closed, but by accident of not understanding the path rather than by intent. Resolution now expands it to the real directory, which is inside the approved root, so accepting it is correct. Verified on Windows, all ten cases: accept a real exe under the approved root accept 8.3 short name resolving into the approved root reject exe reached through a junction pointing out of the root reject alternate data stream on a file in the root reject traversal back out of the root reject UNC path reject sibling directory sharing the root's prefix reject wrong extension reject file that does not exist reject empty path The probe is committed as agent/ShopDbEndpointAgent.PolicyProbe so this is repeatable. It is NOT in CI and its README says why: junctions, alternate data streams and 8.3 names do not exist on Linux, and a suite that skipped every case it exists for would report green and mean nothing. This repo has been bitten by that shape more than once already. Scope: product --- .../LaunchPathPolicy.cs | 43 ++++++-- .../Program.cs | 97 +++++++++++++++++++ .../ShopDbEndpointAgent.PolicyProbe/README.md | 33 +++++++ .../ShopDbEndpointAgent.PolicyProbe.csproj | 20 ++++ agent/ShopDbEndpointAgent.slnx | 2 + 5 files changed, 189 insertions(+), 6 deletions(-) create mode 100644 agent/ShopDbEndpointAgent.PolicyProbe/Program.cs create mode 100644 agent/ShopDbEndpointAgent.PolicyProbe/README.md create mode 100644 agent/ShopDbEndpointAgent.PolicyProbe/ShopDbEndpointAgent.PolicyProbe.csproj diff --git a/agent/ShopDbEndpointAgent.Core/LaunchPathPolicy.cs b/agent/ShopDbEndpointAgent.Core/LaunchPathPolicy.cs index bd4f6c11..94755761 100644 --- a/agent/ShopDbEndpointAgent.Core/LaunchPathPolicy.cs +++ b/agent/ShopDbEndpointAgent.Core/LaunchPathPolicy.cs @@ -134,7 +134,15 @@ public sealed class LaunchPathPolicy // catch. private static string ResolveFinalPath(string resolved) { - var name = System.IO.File.ResolveLinkTarget(resolved, returnFinalTarget: true)?.FullName; + string? name; + try + { + name = System.IO.File.ResolveLinkTarget(resolved, returnFinalTarget: true)?.FullName; + } + catch + { + name = null; + } if (!string.IsNullOrWhiteSpace(name)) { // ResolveLinkTarget resolves the file itself; run it back through @@ -155,21 +163,44 @@ public sealed class LaunchPathPolicy // resolving only the leaf would miss the case this exists to catch. private static string ResolveDirectoryChain(string directory) { - var resolvedDirectory = System.IO.Directory - .ResolveLinkTarget(directory, returnFinalTarget: true)?.FullName; - if (!string.IsNullOrWhiteSpace(resolvedDirectory)) - return resolvedDirectory; - + // Stop at the volume root. Walking into it and asking whether "C:\" is + // a link throws DirectoryNotFoundException, which the caller turned + // into a rejection - so the first version of this rejected every + // legitimate path as well as the malicious ones. A fail-closed gate + // that also fails the normal case is just an outage. Caught in the VM, + // not here. var parent = Path.GetDirectoryName(directory); if (string.IsNullOrEmpty(parent) || parent == directory) return directory; + var resolvedDirectory = TryResolveLink(directory); + if (resolvedDirectory is not null) + return resolvedDirectory; + var realParent = ResolveDirectoryChain(parent); return realParent == parent ? directory : Path.Combine(realParent, Path.GetFileName(directory)); } + // A component's link target, or null when it is not a link OR cannot be + // asked. Never throws: "cannot tell" must not be reported as "is a link", + // and the caller re-checks the assembled path against the roots regardless, + // so an unresolvable component cannot widen what is accepted. + private static string? TryResolveLink(string path) + { + try + { + var target = System.IO.Directory + .ResolveLinkTarget(path, returnFinalTarget: true)?.FullName; + return string.IsNullOrWhiteSpace(target) ? null : target; + } + catch + { + return null; + } + } + private bool IsUnderApprovedRoot(string resolved) { foreach (var root in approvedRoots) diff --git a/agent/ShopDbEndpointAgent.PolicyProbe/Program.cs b/agent/ShopDbEndpointAgent.PolicyProbe/Program.cs new file mode 100644 index 00000000..bee73158 --- /dev/null +++ b/agent/ShopDbEndpointAgent.PolicyProbe/Program.cs @@ -0,0 +1,97 @@ +using ShopDbEndpointAgent.Core; + +// Planted-violation suite for LaunchPathPolicy, run on real Windows because the +// cases that matter (junctions, alternate data streams, 8.3 names) are +// filesystem behaviour, not string behaviour. Linux would prove nothing. +// +// Builds its own fixtures under an approved root, so it exercises the same +// shape as a real shop-floor install rather than a synthetic path. + +int failures = 0; + +void Check(string label, bool expectAccept, string candidate, LaunchPathPolicy policy) +{ + var ok = policy.TryValidate(candidate, out var full, out var reason); + var pass = ok == expectAccept; + if (!pass) failures++; + var verdict = pass ? "PASS" : "FAIL"; + var got = ok ? $"accepted -> {full}" : $"rejected ({reason})"; + Console.WriteLine($"{verdict} {label}"); + Console.WriteLine($" want={(expectAccept ? "accept" : "reject")} got={got}"); +} + +var root = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles), "LppFixture"); +var outside = Path.Combine(Path.GetTempPath(), "LppOutside"); +Directory.CreateDirectory(root); +Directory.CreateDirectory(outside); + +var policy = new LaunchPathPolicy(new[] { root }); + +// A real executable inside the approved root. +var good = Path.Combine(root, "good.exe"); +File.WriteAllBytes(good, new byte[] { 0x4D, 0x5A }); + +// The payload the attacker wants run, planted OUTSIDE the approved root. +var payload = Path.Combine(outside, "payload.exe"); +File.WriteAllBytes(payload, new byte[] { 0x4D, 0x5A }); + +// A text file inside the root, to hang an alternate data stream off. +var carrier = Path.Combine(root, "readme.txt"); +File.WriteAllText(carrier, "notes"); + +Console.WriteLine($"approved root : {root}"); +Console.WriteLine($"outside dir : {outside}"); +Console.WriteLine(); + +Check("a real exe under the approved root", true, good, policy); +Check("wrong extension", false, carrier, policy); +Check("file that does not exist", false, Path.Combine(root, "nope.exe"), policy); +Check("traversal back out of the root", false, + Path.Combine(root, "..", "..", "Windows", "System32", "cmd.exe"), policy); +Check("UNC path", false, @"\\attacker\share\payload.exe", policy); +Check("sibling directory sharing the root's prefix", false, root + "Evil\\payload.exe", policy); +Check("empty path", false, "", policy); + +// --- alternate data stream -------------------------------------------------- +try +{ + File.WriteAllBytes(carrier + ":payload.exe", new byte[] { 0x4D, 0x5A }); + Check("alternate data stream on a file in the root", false, + carrier + ":payload.exe", policy); +} +catch (Exception ex) +{ + Console.WriteLine($"SKIP alternate data stream fixture: {ex.Message}"); +} + +// --- directory junction out of the root ------------------------------------ +var junction = Path.Combine(root, "logs"); +var mk = System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo +{ + FileName = "cmd.exe", + Arguments = $"/c mklink /J \"{junction}\" \"{outside}\"", + UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, +}); +mk!.WaitForExit(); +if (mk.ExitCode == 0) +{ + Check("exe reached through a junction pointing out of the root", false, + Path.Combine(junction, "payload.exe"), policy); +} +else +{ + Console.WriteLine($"SKIP junction fixture: mklink exit {mk.ExitCode} {mk.StandardError.ReadToEnd().Trim()}"); +} + +// --- 8.3 short name --------------------------------------------------------- +// 8.3 short names used to be rejected, because the lexical prefix check could +// not match "C:\PROGRA~1" against "C:\Program Files" - fail closed, but only by +// accident of not understanding the path. Resolution now expands it to the real +// directory, which IS under the approved root, so accepting is correct: the +// check that matters runs on the resolved path either way. +var shortRoot = Path.Combine(@"C:\PROGRA~1", "LppFixture", "good.exe"); +Check("8.3 short name resolving into the approved root", true, shortRoot, policy); + +Console.WriteLine(); +Console.WriteLine(failures == 0 ? "ALL EXPECTATIONS MET" : $"{failures} EXPECTATION(S) NOT MET"); +return failures == 0 ? 0 : 1; diff --git a/agent/ShopDbEndpointAgent.PolicyProbe/README.md b/agent/ShopDbEndpointAgent.PolicyProbe/README.md new file mode 100644 index 00000000..390a67bb --- /dev/null +++ b/agent/ShopDbEndpointAgent.PolicyProbe/README.md @@ -0,0 +1,33 @@ +# LaunchPathPolicy probe + +A planted-violation probe for `LaunchPathPolicy`. It builds real fixtures under +a real approved root - a junction pointing out of it, an alternate data stream, +an 8.3 short name - and asserts what the policy does with each. + +## Why it is not a unit test in CI + +The cases worth testing are filesystem behaviour, not string behaviour: +junctions, alternate data streams and 8.3 short names do not exist on Linux, and +CI runs on Linux runners. A suite that skipped every meaningful case there would +report green and mean nothing, which is the failure shape this repo has already +been bitten by more than once. So it is a probe you run on Windows, by hand, +when the policy changes. + +## Running it + +Publish from the dev box and run it in the win11 VM: + + dotnet publish agent/ShopDbEndpointAgent.PolicyProbe -c Release -r win-x64 \ + --self-contained true -p:PublishSingleFile=true -o /tmp/lpp + +Copy `lpp/lpptest.exe` into the VM and run it as an account that may create a +junction under Program Files. Exit code 0 means every expectation held; it +prints one line per case either way. + +## What it caught + +The first version of the link-resolving hardening recursed to the volume root +and asked whether `C:\` was a link. That throws, and the caller treated a +resolve failure as a rejection - so the policy rejected every legitimate launch +path as well as the malicious ones. It compiled cleanly and read correctly. Only +running it on Windows showed the agent would have failed closed on everything. diff --git a/agent/ShopDbEndpointAgent.PolicyProbe/ShopDbEndpointAgent.PolicyProbe.csproj b/agent/ShopDbEndpointAgent.PolicyProbe/ShopDbEndpointAgent.PolicyProbe.csproj new file mode 100644 index 00000000..9112673f --- /dev/null +++ b/agent/ShopDbEndpointAgent.PolicyProbe/ShopDbEndpointAgent.PolicyProbe.csproj @@ -0,0 +1,20 @@ + + + + Exe + net8.0 + enable + enable + lpptest + LppTest + true + + + + + diff --git a/agent/ShopDbEndpointAgent.slnx b/agent/ShopDbEndpointAgent.slnx index e58da209..1855a388 100644 --- a/agent/ShopDbEndpointAgent.slnx +++ b/agent/ShopDbEndpointAgent.slnx @@ -3,4 +3,6 @@ + +