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
This commit is contained in:
cproudlock
2026-09-02 16:13:01 -04:00
parent e98e15259e
commit 852aec5b7f
5 changed files with 189 additions and 6 deletions

View File

@@ -134,7 +134,15 @@ public sealed class LaunchPathPolicy
// catch. // catch.
private static string ResolveFinalPath(string resolved) 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)) if (!string.IsNullOrWhiteSpace(name))
{ {
// ResolveLinkTarget resolves the file itself; run it back through // 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. // resolving only the leaf would miss the case this exists to catch.
private static string ResolveDirectoryChain(string directory) private static string ResolveDirectoryChain(string directory)
{ {
var resolvedDirectory = System.IO.Directory // Stop at the volume root. Walking into it and asking whether "C:\" is
.ResolveLinkTarget(directory, returnFinalTarget: true)?.FullName; // a link throws DirectoryNotFoundException, which the caller turned
if (!string.IsNullOrWhiteSpace(resolvedDirectory)) // into a rejection - so the first version of this rejected every
return resolvedDirectory; // 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); var parent = Path.GetDirectoryName(directory);
if (string.IsNullOrEmpty(parent) || parent == directory) if (string.IsNullOrEmpty(parent) || parent == directory)
return directory; return directory;
var resolvedDirectory = TryResolveLink(directory);
if (resolvedDirectory is not null)
return resolvedDirectory;
var realParent = ResolveDirectoryChain(parent); var realParent = ResolveDirectoryChain(parent);
return realParent == parent return realParent == parent
? directory ? directory
: Path.Combine(realParent, Path.GetFileName(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) private bool IsUnderApprovedRoot(string resolved)
{ {
foreach (var root in approvedRoots) foreach (var root in approvedRoots)

View File

@@ -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;

View File

@@ -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.

View File

@@ -0,0 +1,20 @@
<Project Sdk="Microsoft.NET.Sdk">
<!-- Planted-violation probe for LaunchPathPolicy. See README.md next to this
file: it is run BY HAND on Windows, not by CI, because the cases it
exists to catch are filesystem behaviour and CI runs on Linux.
net8.0, not net8.0-windows: the policy itself only touches System.IO, and
a plain target cross-publishes from the Linux dev box. -->
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<AssemblyName>lpptest</AssemblyName>
<RootNamespace>LppTest</RootNamespace>
<InvariantGlobalization>true</InvariantGlobalization>
</PropertyGroup>
<ItemGroup>
<Compile Include="../ShopDbEndpointAgent.Core/LaunchPathPolicy.cs" Link="LaunchPathPolicy.cs" />
</ItemGroup>
</Project>

View File

@@ -3,4 +3,6 @@
<Project Path="ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj" /> <Project Path="ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj" />
<Project Path="ShopDbEndpointAgent/ShopDbEndpointAgent.csproj" /> <Project Path="ShopDbEndpointAgent/ShopDbEndpointAgent.csproj" />
<Project Path="ShopDbAdoptionTool/ShopDbAdoptionTool.csproj" /> <Project Path="ShopDbAdoptionTool/ShopDbAdoptionTool.csproj" />
<!-- Run by hand on Windows, never by CI. See its README. -->
<Project Path="ShopDbEndpointAgent.PolicyProbe/ShopDbEndpointAgent.PolicyProbe.csproj" />
</Solution> </Solution>