From 95870a05632a16da137bd48771dde530556a7a0e Mon Sep 17 00:00:00 2001 From: Giulia Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:53:13 +0000 Subject: [PATCH 1/4] Record registry ACE access type and key SDDL RegistryKeyToRegistryObject discarded RegistryAccessRule.AccessControlType, so Allow and Deny ACEs were indistinguishable in collected data and any analysis of registry permissions was unsound. Split the comma joined RegistryRights mask into individual rights, mirroring how FileSystemCollector handles FileSystemRights, and prefix each with the access control type. Entries are now "Allow:SetValue" / "Deny:SetValue", which makes ContainsKey and Contains with DictData usable. The Dictionary> shape is unchanged, and unprefixed rights in existing databases still hydrate. Also populate RegistryObject.PermissionsString, which was declared but never assigned, with the key's SDDL. It is a flat string so rules can match ACE patterns against it with Regex; the Permissions dictionary cannot be matched that way because OAT's regex operation discards dictionary valued fields. --- Lib/Objects/RegistryObject.cs | 9 +++++++++ Lib/Utils/RegistryWalker.cs | 30 +++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/Lib/Objects/RegistryObject.cs b/Lib/Objects/RegistryObject.cs index f7f56a116..d11fcec11 100644 --- a/Lib/Objects/RegistryObject.cs +++ b/Lib/Objects/RegistryObject.cs @@ -31,6 +31,15 @@ public override string Identity public string Key { get; set; } public Dictionary> Permissions { get; set; } = new Dictionary>(); + + /// + /// The key's security descriptor in SDDL form. + /// + /// + /// A flat string, so analysis rules can match ACE patterns against it with Regex. The + /// Permissions dictionary cannot be matched that way: OAT's regex operation discards the + /// dictionary half of a field's values. + /// public string? PermissionsString { get; set; } public int SubkeyCount diff --git a/Lib/Utils/RegistryWalker.cs b/Lib/Utils/RegistryWalker.cs index 8a2bdff2f..68d15bae2 100644 --- a/Lib/Utils/RegistryWalker.cs +++ b/Lib/Utils/RegistryWalker.cs @@ -39,19 +39,39 @@ public static class RegistryWalker try { - foreach (RegistryAccessRule? rule in key.GetAccessControl().GetAccessRules(true, true, typeof(SecurityIdentifier))) + var security = key.GetAccessControl(); + + try + { + regObj.PermissionsString = security.GetSecurityDescriptorSddlForm(AccessControlSections.All); + } + catch (Exception e) + { + Log.Verbose("Failed to get SDDL for {0} ({1}:{2})", regObj.Key, e.GetType(), e.Message); + } + + foreach (RegistryAccessRule? rule in security.GetAccessRules(true, true, typeof(SecurityIdentifier))) { if (rule != null) { string name = AsaHelpers.SidToName(rule.IdentityReference); - if (regObj.Permissions.ContainsKey(name)) + if (!regObj.Permissions.TryGetValue(name, out List? rights)) { - regObj.Permissions[name].Add(rule.RegistryRights.ToString()); + rights = new List(); + regObj.Permissions.Add(name, rights); } - else + + // RegistryRights.ToString() returns a comma joined combined mask. Split it so + // individual rights are matchable, and prefix each with the access control type so + // Allow and Deny are distinguishable. + foreach (var right in rule.RegistryRights.ToString().Split(',')) { - regObj.Permissions.Add(name, new List() { rule.RegistryRights.ToString() }); + var entry = $"{rule.AccessControlType}:{right.Trim()}"; + if (!rights.Contains(entry)) + { + rights.Add(entry); + } } } } From d3c3a3953f6b486119583b784a4da4db7a0373dc Mon Sep 17 00:00:00 2001 From: Giulia Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:53:29 +0000 Subject: [PATCH 2/4] Fix COM server path resolution and handle LocalServer32 ComObjectCollector never expanded environment variables. It explicitly skipped the System32 prefixing branch when a path contained '%' and then passed the raw string to FilePathToFileSystemObject, so every COM server registered under %SystemRoot%, %ProgramData% or similar failed to resolve and silently produced a FileSystemObject with no permissions. Expand environment variables before resolving, and also resolve native object manager prefixes (\??\, \SystemRoot\). Handle LocalServer32 and LocalServer in addition to InprocServer32. Their values are command lines rather than bare paths, so the executable is extracted from the front, preferring a quoted path and then the first token ending in .exe. Remove the InprocServer64 branch. There is no such registry key, so it was dead code. Bitness is selected by the registry view, not the key name, so the resolved binary is now assigned to x86_Binary in the 32-bit view and to x64_Binary in the 64-bit view. Both views are already parsed, so both fields are populated across a run; previously x64_Binary was never set at all. The three near identical inline blocks are replaced by a single ResolveServerBinary helper. Existing quote, whitespace and System32 handling is preserved in RegistryReferenceParser.NormalizePath. --- Lib/Collectors/ComObjectCollector.cs | 133 +++++++------- Lib/Utils/RegistryReferenceParser.cs | 249 +++++++++++++++++++++++++++ 2 files changed, 323 insertions(+), 59 deletions(-) create mode 100644 Lib/Utils/RegistryReferenceParser.cs diff --git a/Lib/Collectors/ComObjectCollector.cs b/Lib/Collectors/ComObjectCollector.cs index 0d0462192..fd3ba9b47 100644 --- a/Lib/Collectors/ComObjectCollector.cs +++ b/Lib/Collectors/ComObjectCollector.cs @@ -45,70 +45,21 @@ public static IEnumerable ParseComObjects(RegistryKey SearchKey, if (RegObj != null) { ComObject comObject = new(RegObj); + var binary = ResolveServerBinary(CurrentKey, View, fsc); - foreach (string ComDetails in CurrentKey.GetSubKeyNames()) + if (binary is not null) { - if (ComDetails.Contains("InprocServer32")) + // Which view the object came from is what determines the bitness of the + // server it registers; the key name does not. InprocServer32 holds the + // 64-bit server in the 64-bit view and the 32-bit server in the 32-bit view + // (where it is redirected to Wow6432Node). + if (View == RegistryView.Registry32) { - var ComKey = CurrentKey.OpenSubKey(ComDetails); - if (ComKey is not null) - { - var obj = RegistryWalker.RegistryKeyToRegistryObject(ComKey, View); - string? BinaryPath32 = null; - - if (obj != null && obj.Values?.TryGetValue("", out BinaryPath32) is bool successful) - { - if (successful && BinaryPath32 != null) - { - // Clean up cases where some extra spaces are thrown into the start - // (breaks our permission checker) - BinaryPath32 = BinaryPath32.Trim(); - // Clean up cases where the binary is quoted (also breaks permission checker) - if (BinaryPath32.StartsWith("\"") && BinaryPath32.EndsWith("\"")) - { - BinaryPath32 = BinaryPath32.AsSpan().Slice(1, BinaryPath32.Length - 2).ToString(); - } - // Unqualified binary name probably comes from Windows\System32 - if (!BinaryPath32.Contains("\\") && !BinaryPath32.Contains("%")) - { - BinaryPath32 = Path.Combine(Environment.SystemDirectory, BinaryPath32.Trim()); - } - - comObject.x86_Binary = fsc.FilePathToFileSystemObject(BinaryPath32.Trim()); - } - } - } + comObject.x86_Binary = binary; } - if (ComDetails.Contains("InprocServer64")) + else { - var ComKey = CurrentKey.OpenSubKey(ComDetails); - if (ComKey is not null) - { - var obj = RegistryWalker.RegistryKeyToRegistryObject(ComKey, View); - string? BinaryPath64 = null; - - if (obj != null && obj.Values?.TryGetValue("", out BinaryPath64) is bool successful) - { - if (successful && BinaryPath64 != null) - { - // Clean up cases where some extra spaces are thrown into the start - // (breaks our permission checker) - BinaryPath64 = BinaryPath64.Trim(); - // Clean up cases where the binary is quoted (also breaks permission checker) - if (BinaryPath64.StartsWith("\"") && BinaryPath64.EndsWith("\"")) - { - BinaryPath64 = BinaryPath64.AsSpan().Slice(1, BinaryPath64.Length - 2).ToString(); - } - // Unqualified binary name probably comes from Windows\System32 - if (!BinaryPath64.Contains("\\") && !BinaryPath64.Contains("%")) - { - BinaryPath64 = Path.Combine(Environment.SystemDirectory, BinaryPath64.Trim()); - } - - comObject.x64_Binary = fsc.FilePathToFileSystemObject(BinaryPath64.Trim()); - } - } - } + comObject.x64_Binary = binary; } } @@ -157,6 +108,70 @@ public override bool CanRunOnPlatform() return RuntimeInformation.IsOSPlatform(OSPlatform.Windows); } + /// + /// The subkeys of a CLSID that name the server implementing it, in the order they are preferred. + /// An in-process server is listed first because a DLL loaded into the calling process is the more + /// interesting load point. + /// + /// + /// There is no InprocServer64 key; bitness is selected by the registry view. + /// + private static readonly string[] ServerSubKeyNames = { "InprocServer32", "LocalServer32", "LocalServer" }; + + /// + /// Reads the default value of the first server subkey present under a CLSID and resolves it to a + /// file on disk. + /// + private static FileSystemObject? ResolveServerBinary(RegistryKey clsidKey, RegistryView view, FileSystemCollector fsc) + { + string[] subKeyNames; + + try + { + subKeyNames = clsidKey.GetSubKeyNames(); + } + catch (Exception e) + { + Log.Verbose("Failed to enumerate subkeys of {0} ({1}:{2})", clsidKey.Name, e.GetType(), e.Message); + return null; + } + + foreach (var serverName in ServerSubKeyNames) + { + var match = Array.Find(subKeyNames, name => name.Equals(serverName, StringComparison.OrdinalIgnoreCase)); + if (match is null) + { + continue; + } + + using var serverKey = clsidKey.OpenSubKey(match); + if (serverKey is null) + { + continue; + } + + var serverObj = RegistryWalker.RegistryKeyToRegistryObject(serverKey, view); + if (serverObj?.Values is null + || !serverObj.Values.TryGetValue(string.Empty, out var raw) + || string.IsNullOrWhiteSpace(raw)) + { + continue; + } + + // LocalServer values are command lines, not bare paths. + var path = serverName.StartsWith("LocalServer", StringComparison.OrdinalIgnoreCase) + ? RegistryReferenceParser.ExtractExecutablePath(raw) + : RegistryReferenceParser.NormalizePath(raw); + + if (path is not null) + { + return fsc.FilePathToFileSystemObject(path); + } + } + + return null; + } + /// /// Execute the Com Collector. We collect the list of Com Objects registered in the registry and /// then examine each binary on the disk they point to. diff --git a/Lib/Utils/RegistryReferenceParser.cs b/Lib/Utils/RegistryReferenceParser.cs new file mode 100644 index 000000000..9eae5bd86 --- /dev/null +++ b/Lib/Utils/RegistryReferenceParser.cs @@ -0,0 +1,249 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. +using System; +using System.Collections.Generic; +using System.IO; +using System.Text.RegularExpressions; + +namespace Microsoft.CST.AttackSurfaceAnalyzer.Utils +{ + /// + /// Cracks file paths and CLSIDs out of raw registry value data and normalizes registry-stored binary + /// references into paths on disk. + /// + /// + /// Extraction runs for every value of every key of every hive in both registry views, so it is pure + /// string manipulation over compiled expressions and never touches the disk. Cheap substring + /// pre-filters keep the regexes from running at all for the overwhelming majority of values + /// (base64-encoded REG_BINARY blobs, numbers, and plain words contain none of the trigger + /// characters). + /// + public static class RegistryReferenceParser + { + /// + /// Values longer than this are not scanned. Registry values can hold megabytes of binary data and + /// no load point reference lives past this bound. + /// + public const int MaxScannedValueLength = 8192; + + /// + /// The most references of each kind taken from a single value. + /// + public const int MaxReferencesPerValue = 32; + + /// + /// Extracts CLSID-shaped GUIDs from raw registry value data, normalized to braced uppercase form. + /// + public static IEnumerable ExtractClsids(string? value) + { + if (value is null || value.Length == 0 || value.Length > MaxScannedValueLength) + { + return Array.Empty(); + } + + // Every GUID shape we accept contains hyphens; bail before touching the regex engine if there + // are none. + if (value.IndexOf('-') < 0) + { + return Array.Empty(); + } + + List results = new(); + HashSet seen = new(StringComparer.OrdinalIgnoreCase); + + try + { + for (var match = ClsidRegex.Match(value); match.Success; match = match.NextMatch()) + { + var clsid = $"{{{match.Groups["guid"].Value.ToUpperInvariant()}}}"; + if (seen.Add(clsid)) + { + results.Add(clsid); + if (results.Count >= MaxReferencesPerValue) + { + break; + } + } + } + } + catch (RegexMatchTimeoutException) + { + return results; + } + + return results; + } + + /// + /// Extracts rooted file paths (drive-qualified, UNC, or environment-variable rooted) from raw + /// registry value data. Results are environment-expanded and normalized. + /// + public static IEnumerable ExtractPaths(string? value) + { + if (value is null || value.Length == 0 || value.Length > MaxScannedValueLength) + { + return Array.Empty(); + } + + // Every path shape we accept is rooted by a drive letter, a UNC prefix, or an environment + // variable, so it must contain a backslash or a percent sign. + if (value.IndexOf('\\') < 0 && value.IndexOf('%') < 0) + { + return Array.Empty(); + } + + List results = new(); + HashSet seen = new(StringComparer.OrdinalIgnoreCase); + + try + { + for (var match = PathRegex.Match(value); match.Success; match = match.NextMatch()) + { + var path = NormalizePath(match.Value); + if (path is not null && seen.Add(path)) + { + results.Add(path); + if (results.Count >= MaxReferencesPerValue) + { + break; + } + } + } + } + catch (RegexMatchTimeoutException) + { + return results; + } + + return results; + } + + /// + /// Pulls the executable out of a command line, as stored in LocalServer32 or a service ImagePath. + /// + /// + /// An unquoted path containing spaces is genuinely ambiguous to the loader as well, so the first + /// token ending in .exe is preferred before falling back to splitting on whitespace. + /// + public static string? ExtractExecutablePath(string? commandLine) + { + if (string.IsNullOrWhiteSpace(commandLine)) + { + return null; + } + + var value = commandLine.Trim(); + + if (value[0] == '"') + { + var end = value.IndexOf('"', 1); + return NormalizePath(end > 1 ? value.Substring(1, end - 1) : value.Trim('"')); + } + + try + { + var match = ExecutableExtensionRegex.Match(value); + if (match.Success) + { + return NormalizePath(value.Substring(0, match.Index + match.Length)); + } + } + catch (RegexMatchTimeoutException) + { + // Fall through to whitespace splitting. + } + + var space = value.IndexOf(' '); + return NormalizePath(space > 0 ? value.Substring(0, space) : value); + } + + /// + /// Normalizes a registry-stored binary reference into a path on disk: strips surrounding quotes, + /// expands environment variables, resolves native object-manager prefixes, and qualifies bare + /// binary names against System32 the way the loader would. + /// + public static string? NormalizePath(string? raw) + { + if (string.IsNullOrWhiteSpace(raw)) + { + return null; + } + + var path = raw.Trim(); + + // Quoted paths break permission lookups downstream. + if (path.Length > 1 && path[0] == '"' && path[path.Length - 1] == '"') + { + path = path.Substring(1, path.Length - 2).Trim(); + } + + if (path.Length == 0) + { + return null; + } + + try + { + path = Environment.ExpandEnvironmentVariables(path); + } + catch (Exception) + { + // A malformed value is left as-is rather than dropped. + } + + if (path.StartsWith(@"\??\", StringComparison.Ordinal)) + { + path = path.Substring(4); + } + else if (path.StartsWith(@"\SystemRoot\", StringComparison.OrdinalIgnoreCase)) + { + path = Path.Combine(SystemRoot, path.Substring(@"\SystemRoot\".Length)); + } + + path = path.Trim(); + + if (path.Length == 0) + { + return null; + } + + // An unqualified binary name is resolved by the loader out of System32. + if (path.IndexOf('\\') < 0 && path.IndexOf('/') < 0 && path.IndexOf('%') < 0) + { + path = Path.Combine(Environment.SystemDirectory, path); + } + + return path; + } + + private static string SystemRoot + { + get + { + var windows = Environment.GetFolderPath(Environment.SpecialFolder.Windows); + return string.IsNullOrEmpty(windows) ? @"C:\Windows" : windows; + } + } + + private static readonly TimeSpan RegexTimeout = TimeSpan.FromMilliseconds(100); + + private static readonly Regex ClsidRegex = new( + @"\{?(?[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12})\}?", + RegexOptions.Compiled | RegexOptions.CultureInvariant, + RegexTimeout); + + private static readonly Regex ExecutableExtensionRegex = new( + @"\.exe\b", + RegexOptions.Compiled | RegexOptions.CultureInvariant | RegexOptions.IgnoreCase, + RegexTimeout); + + /// + /// A drive-letter, UNC, or environment-variable rooted path running lazily up to a file + /// extension. The trailing lookahead forces the extension to end at a natural delimiter so that + /// "C:\a\b.check" is not truncated to "C:\a\b.c". + /// + private static readonly Regex PathRegex = new( + @"(?:[A-Za-z]:\\|\\\\|%[A-Za-z_][A-Za-z0-9_()]{0,63}%\\)[^""<>|\r\n\t]*?\.[A-Za-z0-9]{1,8}(?![^\s""',;)\]])", + RegexOptions.Compiled | RegexOptions.CultureInvariant, + RegexTimeout); + } +} From 66600d712dca0a9626008bb2732531728bb6ee72 Mon Sep 17 00:00:00 2001 From: Giulia Stocco <98900+gfs@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:54:04 +0000 Subject: [PATCH 3/4] Detect lay-and-wait DLL load privilege escalation via load points Analysis rules are evaluated against the before and after states of a single object and cannot reach a second one, so a rule over a RegistryObject can never ask about the file that key points at. Detecting a load point that unprivileged users can repoint, or that resolves to a binary they can supply, therefore requires the join to happen during collection. Add LoadPointObject, which carries for one collected unit the source registry key and its ACL, the CLSID it resolved through, the resolved target path, the target's ACL, and flat boolean and string summaries a rule can test directly: SourceKeyUserWritable, TargetUserWritable, TargetExists, TargetAclSource and TargetAclUnavailable. Flat fields are deliberate; dotted field navigation is shallow and regex cannot match dictionary valued fields. Add LoadPointCollector, driven by a list of LoadPointDefinition rather than a chain of ifs. It covers COM servers, StaticPluginMap under InstallService\State, AppInit_DLLs and services; adding coverage means adding an entry. A load point naming a CLSID is followed through Classes\CLSID\{guid}\InprocServer32 to the binary, which is the indirection the whole feature exists to resolve. A missing target is represented explicitly and kept distinct from a failed ACL read, because a nonexistent DLL at a path an unprivileged user can write is the exploitable condition. Where the target is missing the ACL of the nearest existing parent directory is captured instead, and TargetAclSource says which was used. PermissionUtils holds the single definition of user writable, evaluated against Everyone, INTERACTIVE, Authenticated Users and BUILTIN\Users by both SID and account name, with Deny taking precedence over Allow. RegistryObject gains ReferencedPaths and ReferencedClsids, cracked out of the key's values at collection time and environment expanded. They are List so Regex, Contains, StartsWith and EndsWith all work on them. Extraction is pure string manipulation over compiled expressions with substring pre-filters, a match timeout and length and count caps, since it runs for every value of every key of every hive in both views. Ship four rules over the new type covering a writable source key, a writable target, a missing target in a writable directory, and both ends under unprivileged control. Severities match the existing rules, which top out at WARNING. Tests cover reference extraction, Deny precedence, nearest existing parent resolution, serialization round trips, and rule matching against a synthetic object in the shape of CVE-2026-50343 plus benign and unreadable-ACL negatives. False positives are the main risk to this feature, so the negative cases are asserted explicitly. Registry and COM collection remain Windows only and are gated accordingly. --- Cli/AttackSurfaceAnalyzerClient.cs | 9 + .../LoadPointCollectorOptions.razor | 10 + Cli/Components/CollectorOptionsRazor.razor | 1 + Lib/Collectors/LoadPointCollector.cs | 546 ++++++++++++++++++ Lib/Objects/AsaRule.cs | 3 + Lib/Objects/CommandOptions.cs | 4 + Lib/Objects/LoadPointObject.cs | 131 +++++ Lib/Objects/RegistryObject.cs | 15 + Lib/Objects/Types.cs | 7 +- Lib/Utils/JsonUtils.cs | 4 + Lib/Utils/PermissionUtils.cs | 311 ++++++++++ Lib/Utils/RegistryWalker.cs | 56 +- Tests/AsaAnalyzerTests.cs | 124 ++++ Tests/CollectorTests.cs | 40 ++ Tests/HydrationTests.cs | 66 +++ Tests/LoadPointUtilsTests.cs | 246 ++++++++ analyses.json | 107 +++- 17 files changed, 1676 insertions(+), 4 deletions(-) create mode 100644 Cli/Components/CollectorOptions/LoadPointCollectorOptions.razor create mode 100644 Lib/Collectors/LoadPointCollector.cs create mode 100644 Lib/Objects/LoadPointObject.cs create mode 100644 Lib/Utils/PermissionUtils.cs create mode 100644 Tests/LoadPointUtilsTests.cs diff --git a/Cli/AttackSurfaceAnalyzerClient.cs b/Cli/AttackSurfaceAnalyzerClient.cs index 64d2f9184..7c9d93faa 100644 --- a/Cli/AttackSurfaceAnalyzerClient.cs +++ b/Cli/AttackSurfaceAnalyzerClient.cs @@ -1509,6 +1509,10 @@ public static ASA_ERROR RunCollectCommand(CollectCommandOptions opts) case RESULT_TYPE.WIFI: opts.EnableWifiCollector = true; break; + + case RESULT_TYPE.LOADPOINT: + opts.EnableLoadPointCollector = true; + break; } } } @@ -1588,6 +1592,11 @@ public static ASA_ERROR RunCollectCommand(CollectCommandOptions opts) collectors.Add(new WifiCollector(opts, defaultChangeHandler)); dict.Add(RESULT_TYPE.WIFI); } + if (opts.EnableLoadPointCollector || (opts.EnableAllCollectors && RuntimeInformation.IsOSPlatform(OSPlatform.Windows))) + { + collectors.Add(new LoadPointCollector(opts, defaultChangeHandler)); + dict.Add(RESULT_TYPE.LOADPOINT); + } if (collectors.Count == 0) { diff --git a/Cli/Components/CollectorOptions/LoadPointCollectorOptions.razor b/Cli/Components/CollectorOptions/LoadPointCollectorOptions.razor new file mode 100644 index 000000000..0a820c8c2 --- /dev/null +++ b/Cli/Components/CollectorOptions/LoadPointCollectorOptions.razor @@ -0,0 +1,10 @@ +@inject Microsoft.CST.AttackSurfaceAnalyzer.Cli.AppData appData + +
+
+
+ + +
+
+
diff --git a/Cli/Components/CollectorOptionsRazor.razor b/Cli/Components/CollectorOptionsRazor.razor index 1f321ab35..0ac296108 100644 --- a/Cli/Components/CollectorOptionsRazor.razor +++ b/Cli/Components/CollectorOptionsRazor.razor @@ -23,6 +23,7 @@
+ diff --git a/Lib/Collectors/LoadPointCollector.cs b/Lib/Collectors/LoadPointCollector.cs new file mode 100644 index 000000000..05677a843 --- /dev/null +++ b/Lib/Collectors/LoadPointCollector.cs @@ -0,0 +1,546 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. +using Microsoft.CST.AttackSurfaceAnalyzer.Objects; +using Microsoft.CST.AttackSurfaceAnalyzer.Utils; +using Microsoft.Win32; +using Serilog; +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; +using System.Runtime.Versioning; +using System.Security.AccessControl; +using System.Threading; + +namespace Microsoft.CST.AttackSurfaceAnalyzer.Collectors +{ + /// + /// How the data stored in a load point's registry value should be read. + /// + public enum LoadPointTargetKind + { + /// + /// The value holds one or more file paths. + /// + Path, + + /// + /// The value holds a command line; the executable is taken from the front of it. + /// + CommandLine, + + /// + /// The value references CLSIDs, which are resolved through Classes\CLSID to a binary. + /// + Clsid, + + /// + /// CLSIDs if the value contains any, otherwise file paths. + /// + Auto + } + + /// + /// A value, or set of values, under a load point key that names code to load. + /// + public sealed class LoadPointValueSource + { + public LoadPointValueSource(string? subKey, string? valueName, LoadPointTargetKind kind) + { + SubKey = subKey; + ValueName = valueName; + Kind = kind; + } + + /// + /// Subkey holding the value, relative to the unit being examined. Null means the unit key itself. + /// + public string? SubKey { get; } + + /// + /// The value to read. Empty string is the key's default value; null means every value. + /// + public string? ValueName { get; } + + public LoadPointTargetKind Kind { get; } + } + + /// + /// A place in the registry the operating system reads to decide what code to load. + /// + public sealed class LoadPointDefinition + { + public LoadPointDefinition(string name, RegistryHive hive, string keyPath, bool enumerateSubKeys, params LoadPointValueSource[] sources) + { + Name = name; + Hive = hive; + KeyPath = keyPath; + EnumerateSubKeys = enumerateSubKeys; + Sources = new ReadOnlyCollection(sources); + } + + /// + /// Reported as . + /// + public string Name { get; } + + public RegistryHive Hive { get; } + + public string KeyPath { get; } + + /// + /// When true each immediate subkey of is a load point in its own right, + /// as with a CLSID or a service. When false the key itself is the load point. + /// + public bool EnumerateSubKeys { get; } + + public IReadOnlyList Sources { get; } + } + + /// + /// Collects registry load points joined to the binaries they resolve to, so that an unprivileged + /// user's ability to modify either end can be asserted by a single analysis rule. + /// + public class LoadPointCollector : BaseCollector + { + public LoadPointCollector(CollectorOptions? opts = null, Action? changeHandler = null, IEnumerable? definitions = null) + : base(opts, changeHandler) + { + _definitions = definitions?.ToList() ?? DefaultDefinitions.ToList(); + } + + /// + /// The load points scanned unless the caller supplies its own set. Adding coverage is a matter of + /// adding an entry here rather than adding a branch to the collector. + /// + public static IReadOnlyList DefaultDefinitions { get; } = new ReadOnlyCollection(new[] + { + new LoadPointDefinition("ComServer", RegistryHive.LocalMachine, ClsidKeyPath, true, + new LoadPointValueSource("InprocServer32", string.Empty, LoadPointTargetKind.Path), + new LoadPointValueSource("LocalServer32", string.Empty, LoadPointTargetKind.CommandLine), + new LoadPointValueSource("LocalServer", string.Empty, LoadPointTargetKind.CommandLine)), + + // The key that CVE-2026-50343 abuses: its DACL grants INTERACTIVE SetValue and CreateSubKey, and + // the plugin IDs it maps are CoCreateInstance'd by a SYSTEM svchost. + new LoadPointDefinition("StaticPluginMap", RegistryHive.LocalMachine, @"SOFTWARE\Microsoft\Windows\CurrentVersion\InstallService\State", false, + new LoadPointValueSource(null, null, LoadPointTargetKind.Auto)), + + new LoadPointDefinition("AppInit_DLLs", RegistryHive.LocalMachine, @"SOFTWARE\Microsoft\Windows NT\CurrentVersion\Windows", false, + new LoadPointValueSource(null, "AppInit_DLLs", LoadPointTargetKind.Path)), + + new LoadPointDefinition("Service", RegistryHive.LocalMachine, @"SYSTEM\CurrentControlSet\Services", true, + new LoadPointValueSource(null, "ImagePath", LoadPointTargetKind.CommandLine), + new LoadPointValueSource("Parameters", "ServiceDll", LoadPointTargetKind.Path)), + }); + + /// + /// Registry load points only exist on Windows. + /// + public override bool CanRunOnPlatform() => RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + + internal override void ExecuteInternal(CancellationToken cancellationToken) + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + + foreach (var view in new[] { RegistryView.Registry64, RegistryView.Registry32 }) + { + foreach (var definition in _definitions) + { + if (cancellationToken.IsCancellationRequested) + { + return; + } + + foreach (var loadPoint in ParseDefinition(definition, view, cancellationToken)) + { + if (cancellationToken.IsCancellationRequested) + { + return; + } + + HandleChange(loadPoint); + } + } + } + } + + /// + /// Expands a single load point definition in one registry view. + /// + [SupportedOSPlatform("windows")] + public IEnumerable ParseDefinition(LoadPointDefinition definition, RegistryView view, CancellationToken cancellationToken = default) + { + if (definition is null) + { + throw new ArgumentNullException(nameof(definition)); + } + + RegistryKey? baseKey = null; + RegistryKey? rootKey = null; + + try + { + baseKey = RegistryKey.OpenBaseKey(definition.Hive, view); + rootKey = baseKey.OpenSubKey(definition.KeyPath); + } + catch (Exception e) + { + Log.Verbose("Failed to open {0}\\{1} ({2}:{3})", definition.Hive, definition.KeyPath, e.GetType(), e.Message); + } + + if (rootKey is null) + { + baseKey?.Dispose(); + return Array.Empty(); + } + + List results = new(); + var fsc = new FileSystemCollector(new CollectorOptions() { SingleThread = true }); + + try + { + if (definition.EnumerateSubKeys) + { + foreach (var subKeyName in SafeGetSubKeyNames(rootKey)) + { + if (cancellationToken.IsCancellationRequested) + { + break; + } + + using var unit = SafeOpenSubKey(rootKey, subKeyName); + if (unit is not null) + { + results.AddRange(ParseUnit(definition, unit, view, baseKey, fsc)); + } + } + } + else + { + results.AddRange(ParseUnit(definition, rootKey, view, baseKey, fsc)); + } + } + finally + { + rootKey.Dispose(); + baseKey?.Dispose(); + } + + return results; + } + + [SupportedOSPlatform("windows")] + private static IEnumerable ParseUnit(LoadPointDefinition definition, RegistryKey unit, RegistryView view, RegistryKey? hiveRoot, FileSystemCollector fsc) + { + List results = new(); + + foreach (var source in definition.Sources) + { + RegistryKey? valueKey = unit; + RegistryKey? opened = null; + + if (source.SubKey is not null) + { + opened = SafeOpenSubKey(unit, source.SubKey); + valueKey = opened; + } + + if (valueKey is null) + { + continue; + } + + try + { + var sourceObj = RegistryWalker.RegistryKeyToRegistryObject(valueKey, view); + if (sourceObj?.Values is null || sourceObj.Values.Count == 0) + { + continue; + } + + var sourceWritable = PermissionUtils.IsUserWritable(sourceObj.Permissions); + + foreach (var entry in SelectValues(sourceObj.Values, source.ValueName)) + { + foreach (var loadPoint in ResolveReferences(definition, source, sourceObj, sourceWritable, entry.Key, entry.Value, view, hiveRoot, fsc)) + { + results.Add(loadPoint); + } + } + } + catch (Exception e) + { + Log.Verbose("Failed to parse load point {0} under {1} ({2}:{3})", definition.Name, unit.Name, e.GetType(), e.Message); + } + finally + { + opened?.Dispose(); + } + } + + return results; + } + + private static IEnumerable> SelectValues(Dictionary values, string? valueName) + { + if (valueName is null) + { + return values; + } + + return values.TryGetValue(valueName, out var data) + ? new[] { new KeyValuePair(valueName, data) } + : Array.Empty>(); + } + + [SupportedOSPlatform("windows")] + private static IEnumerable ResolveReferences( + LoadPointDefinition definition, + LoadPointValueSource source, + RegistryObject sourceObj, + bool sourceWritable, + string valueName, + string valueData, + RegistryView view, + RegistryKey? hiveRoot, + FileSystemCollector fsc) + { + if (string.IsNullOrWhiteSpace(valueData)) + { + yield break; + } + + var kind = source.Kind; + IReadOnlyList clsids = Array.Empty(); + + if (kind is LoadPointTargetKind.Clsid or LoadPointTargetKind.Auto) + { + clsids = RegistryReferenceParser.ExtractClsids(valueData).ToList(); + if (clsids.Count > 0) + { + kind = LoadPointTargetKind.Clsid; + } + else if (kind == LoadPointTargetKind.Auto) + { + kind = LoadPointTargetKind.Path; + } + } + + var origin = $"{sourceObj.Key}:{(string.IsNullOrEmpty(valueName) ? "(default)" : valueName)}"; + + if (kind == LoadPointTargetKind.Clsid) + { + foreach (var clsid in clsids) + { + var (path, chain) = ResolveClsid(clsid, hiveRoot, view); + var loadPoint = NewLoadPoint(definition, sourceObj, sourceWritable, valueName, valueData, view); + loadPoint.TargetClsid = clsid; + loadPoint.ResolutionChain.Add(origin); + loadPoint.ResolutionChain.AddRange(chain); + PopulateTarget(loadPoint, path, fsc); + yield return loadPoint; + } + + yield break; + } + + IEnumerable paths = kind == LoadPointTargetKind.CommandLine + ? new[] { RegistryReferenceParser.ExtractExecutablePath(valueData) } + : RegistryReferenceParser.ExtractPaths(valueData); + + var any = false; + + foreach (var path in paths) + { + if (path is null) + { + continue; + } + + any = true; + var loadPoint = NewLoadPoint(definition, sourceObj, sourceWritable, valueName, valueData, view); + loadPoint.ResolutionChain.Add(origin); + PopulateTarget(loadPoint, path, fsc); + yield return loadPoint; + } + + // A value that is a bare, unrooted binary name still names something the loader will find. + if (!any && kind == LoadPointTargetKind.Path) + { + var fallback = RegistryReferenceParser.NormalizePath(valueData); + if (fallback is not null) + { + var loadPoint = NewLoadPoint(definition, sourceObj, sourceWritable, valueName, valueData, view); + loadPoint.ResolutionChain.Add(origin); + PopulateTarget(loadPoint, fallback, fsc); + yield return loadPoint; + } + } + } + + /// + /// Follows a CLSID to the binary that implements it. This indirection is the point of the + /// collector: the key an attacker can write names a CLSID, and only the CLSID names the DLL. + /// + [SupportedOSPlatform("windows")] + private static (string? Path, List Chain) ResolveClsid(string clsid, RegistryKey? hiveRoot, RegistryView view) + { + List chain = new(); + + if (hiveRoot is null) + { + return (null, chain); + } + + foreach (var serverName in ClsidServerSubKeys) + { + var keyPath = $@"{ClsidKeyPath}\{clsid}\{serverName}"; + using var serverKey = SafeOpenSubKey(hiveRoot, keyPath); + if (serverKey is null) + { + continue; + } + + var serverObj = RegistryWalker.RegistryKeyToRegistryObject(serverKey, view); + if (serverObj?.Values is null + || !serverObj.Values.TryGetValue(string.Empty, out var raw) + || string.IsNullOrWhiteSpace(raw)) + { + continue; + } + + var path = serverName.StartsWith("LocalServer", StringComparison.OrdinalIgnoreCase) + ? RegistryReferenceParser.ExtractExecutablePath(raw) + : RegistryReferenceParser.NormalizePath(raw); + + chain.Add($@"{hiveRoot.Name}\{keyPath}:(default)"); + return (path, chain); + } + + chain.Add($@"{hiveRoot.Name}\{ClsidKeyPath}\{clsid} (unresolved)"); + return (null, chain); + } + + private static LoadPointObject NewLoadPoint(LoadPointDefinition definition, RegistryObject sourceObj, bool sourceWritable, string valueName, string valueData, RegistryView view) + => new(definition.Name, sourceObj) + { + SourceValueName = valueName, + SourceValueData = valueData.Length > MaxRetainedValueLength + ? valueData.Substring(0, MaxRetainedValueLength) + : valueData, + SourceKeyUserWritable = sourceWritable, + View = view, + }; + + /// + /// Records what the load point resolves to and who can write it. A target that does not exist is + /// reported as such and judged by the ACL of the directory that would receive it, which is the + /// exploitable shape this collector exists to find. + /// + [SupportedOSPlatform("windows")] + private static void PopulateTarget(LoadPointObject loadPoint, string? path, FileSystemCollector fsc) + { + loadPoint.TargetPath = path; + + if (string.IsNullOrEmpty(path)) + { + loadPoint.TargetAclSource = "None"; + return; + } + + try + { + loadPoint.TargetExists = File.Exists(path) || Directory.Exists(path); + } + catch (Exception e) + { + Log.Verbose("Failed to test existence of {0} ({1}:{2})", path, e.GetType(), e.Message); + } + + if (loadPoint.TargetExists) + { + loadPoint.Target = fsc.FilePathToFileSystemObject(path!); + loadPoint.TargetAclSource = "Target"; + loadPoint.TargetUserWritable = TryIsUserWritable(path!, out var unavailable); + loadPoint.TargetAclUnavailable = unavailable; + return; + } + + var parent = PermissionUtils.NearestExistingParent(path); + if (parent is null) + { + loadPoint.TargetAclSource = "None"; + loadPoint.TargetAclUnavailable = true; + return; + } + + loadPoint.NearestExistingParentPath = parent; + loadPoint.NearestExistingParent = fsc.FilePathToFileSystemObject(parent); + loadPoint.TargetAclSource = "NearestExistingParent"; + loadPoint.TargetUserWritable = TryIsUserWritable(parent, out var parentUnavailable); + loadPoint.TargetAclUnavailable = parentUnavailable; + } + + [SupportedOSPlatform("windows")] + private static bool TryIsUserWritable(string path, out bool unavailable) + { + try + { + FileSystemSecurity security = Directory.Exists(path) + ? new DirectoryInfo(path).GetAccessControl(AccessControlSections.Access) + : new FileInfo(path).GetAccessControl(AccessControlSections.Access); + + unavailable = false; + return PermissionUtils.IsUserWritable(security); + } + catch (Exception e) + { + Log.Verbose("Failed to read ACL of {0} ({1}:{2})", path, e.GetType(), e.Message); + unavailable = true; + return false; + } + } + + private static string[] SafeGetSubKeyNames(RegistryKey key) + { + try + { + return key.GetSubKeyNames(); + } + catch (Exception e) + { + Log.Verbose("Failed to enumerate subkeys of {0} ({1}:{2})", key.Name, e.GetType(), e.Message); + return Array.Empty(); + } + } + + private static RegistryKey? SafeOpenSubKey(RegistryKey key, string name) + { + try + { + return key.OpenSubKey(name); + } + catch (Exception e) + { + Log.Verbose("Failed to open {0}\\{1} ({2}:{3})", key.Name, name, e.GetType(), e.Message); + return null; + } + } + + private const string ClsidKeyPath = @"SOFTWARE\Classes\CLSID"; + + private static readonly string[] ClsidServerSubKeys = { "InprocServer32", "LocalServer32", "LocalServer" }; + + /// + /// Load point values such as StaticPluginMap can be large; only enough to identify what was + /// written is retained. + /// + private const int MaxRetainedValueLength = 512; + + private readonly List _definitions; + } +} diff --git a/Lib/Objects/AsaRule.cs b/Lib/Objects/AsaRule.cs index 7fb8abc7b..36e2bb0df 100644 --- a/Lib/Objects/AsaRule.cs +++ b/Lib/Objects/AsaRule.cs @@ -70,6 +70,9 @@ public RESULT_TYPE ResultType case RESULT_TYPE.KEY: return typeof(CryptographicKeyObject).Name; + case RESULT_TYPE.LOADPOINT: + return typeof(LoadPointObject).Name; + case RESULT_TYPE.LOG: return typeof(EventLogObject).Name; diff --git a/Lib/Objects/CommandOptions.cs b/Lib/Objects/CommandOptions.cs index b9df8eb54..ede82d485 100644 --- a/Lib/Objects/CommandOptions.cs +++ b/Lib/Objects/CommandOptions.cs @@ -32,6 +32,7 @@ public static CollectCommandOptions FromCollectorOptions(CollectorOptions opts) EnableFileSystemCollector = opts.EnableFileSystemCollector, EnableFirewallCollector = opts.EnableFirewallCollector, EnableKeyCollector = opts.EnableKeyCollector, + EnableLoadPointCollector = opts.EnableLoadPointCollector, EnableNetworkPortCollector = opts.EnableNetworkPortCollector, EnableProcessCollector = opts.EnableProcessCollector, EnableRegistryCollector = opts.EnableRegistryCollector, @@ -85,6 +86,9 @@ public class CollectorOptions : CommandOptions [Option('k', "keys", Required = false, HelpText = "Gather information about the cryptographic keys on the system.")] public bool EnableKeyCollector { get; set; } + [Option('L', "load-points", Required = false, HelpText = "Enable the load point collector, which joins registry load points to the binaries they resolve to")] + public bool EnableLoadPointCollector { get; set; } + [Option('p', "network-port", Required = false, HelpText = "Enable the network port collector")] public bool EnableNetworkPortCollector { get; set; } diff --git a/Lib/Objects/LoadPointObject.cs b/Lib/Objects/LoadPointObject.cs new file mode 100644 index 000000000..8a86359e5 --- /dev/null +++ b/Lib/Objects/LoadPointObject.cs @@ -0,0 +1,131 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. +using Microsoft.CST.AttackSurfaceAnalyzer.Types; +using Microsoft.Win32; +using System.Collections.Generic; + +namespace Microsoft.CST.AttackSurfaceAnalyzer.Objects +{ + /// + /// A place where the operating system is told to load code, joined at collection time to the binary + /// it resolves to. + /// + /// + /// + /// Analysis rules are evaluated against the before and after states of a single object and cannot + /// reach a second one, so a rule over a can never ask about the + /// file that key points at. This object performs that join during collection: it carries the + /// source key and its ACL, the resolved target path, the target's ACL, and flat summary flags a + /// rule can test directly. + /// + /// + /// Summary fields are deliberately booleans and strings. Dotted field navigation in rules is + /// shallow and regex cannot match dictionary-typed fields, so nested structures are not + /// interrogable. + /// + /// + public class LoadPointObject : CollectObject + { + public LoadPointObject(string LoadPointType, RegistryObject SourceKey) + { + this.LoadPointType = LoadPointType; + this.SourceKey = SourceKey; + } + + public override RESULT_TYPE ResultType => RESULT_TYPE.LOADPOINT; + + /// + /// A load point is identified by the key and value it came from together with what that resolved + /// to, so that two plugins registered under one key remain distinct objects. + /// + public override string Identity => $"{LoadPointType}_{SourceKey.Identity}_{SourceValueName}_{TargetClsid}_{TargetPath}"; + + /// + /// Which kind of load point this is, e.g. ComServer, StaticPluginMap, AppInit_DLLs, Service. + /// + public string LoadPointType { get; set; } + + /// + /// The registry key that names the code to load, including its ACL. + /// + public RegistryObject SourceKey { get; set; } + + /// + /// The value under this load point came from. Empty for a key's default + /// value. + /// + public string? SourceValueName { get; set; } + + /// + /// The raw value data before expansion, retained so a rule can see what was actually written. + /// + public string? SourceValueData { get; set; } + + /// + /// True when can be modified by an unprivileged user, meaning an + /// attacker can repoint this load point at code of their choosing. + /// + public bool SourceKeyUserWritable { get; set; } + + /// + /// The steps taken to get from the source key to the target binary, for triage. + /// + public List ResolutionChain { get; set; } = new List(); + + /// + /// The CLSID this load point resolved through, if it referenced one. + /// + public string? TargetClsid { get; set; } + + /// + /// The resolved path of the binary that will be loaded. + /// + public string? TargetPath { get; set; } + + /// + /// The target binary, including its ACL. Null when the target does not exist. + /// + public FileSystemObject? Target { get; set; } + + /// + /// Whether the target binary is present on disk. A missing target at a path an unprivileged user + /// can write is the exploitable case, so this is asserted directly rather than inferred from a + /// null Target, which is also what a failed collection produces. + /// + public bool TargetExists { get; set; } + + /// + /// True when the target's ACL could not be read at all, so is + /// not a statement about the target's security. + /// + public bool TargetAclUnavailable { get; set; } + + /// + /// Whether describes the target itself or the directory that + /// would receive it: Target, NearestExistingParent, or None. + /// + public string TargetAclSource { get; set; } = "None"; + + /// + /// True when an unprivileged user can write the target binary, or when the target is missing and + /// an unprivileged user can create it in the nearest existing parent directory. + /// + public bool TargetUserWritable { get; set; } + + /// + /// The closest ancestor directory of that exists on disk. Populated + /// when the target is missing, because that directory's ACL is what decides whether an attacker + /// can supply the file. + /// + public string? NearestExistingParentPath { get; set; } + + /// + /// The nearest existing parent directory, including its ACL. + /// + public FileSystemObject? NearestExistingParent { get; set; } + + /// + /// The registry view this load point was collected from. + /// + public RegistryView View { get; set; } + } +} diff --git a/Lib/Objects/RegistryObject.cs b/Lib/Objects/RegistryObject.cs index d11fcec11..adb286692 100644 --- a/Lib/Objects/RegistryObject.cs +++ b/Lib/Objects/RegistryObject.cs @@ -42,6 +42,21 @@ public override string Identity /// public string? PermissionsString { get; set; } + /// + /// CLSID-shaped GUIDs referenced by this key's values, in braced uppercase form. + /// + /// + /// Pre-parsed at collection time because analysis rules cannot follow a reference from one + /// object to another. A List<string> so that Regex, Contains, StartsWith, and EndsWith all + /// work against it. + /// + public List ReferencedClsids { get; set; } = new List(); + + /// + /// File paths referenced by this key's values, environment-expanded and normalized. + /// + public List ReferencedPaths { get; set; } = new List(); + public int SubkeyCount { get { return Subkeys?.Count ?? 0; } diff --git a/Lib/Objects/Types.cs b/Lib/Objects/Types.cs index 1c05dd55d..ea8e834c3 100644 --- a/Lib/Objects/Types.cs +++ b/Lib/Objects/Types.cs @@ -303,7 +303,12 @@ public enum RESULT_TYPE /// /// A wifi network /// - WIFI + WIFI, + + /// + /// See LoadPointObject + /// + LOADPOINT }; /// diff --git a/Lib/Utils/JsonUtils.cs b/Lib/Utils/JsonUtils.cs index 9e66e6282..e1613892a 100644 --- a/Lib/Utils/JsonUtils.cs +++ b/Lib/Utils/JsonUtils.cs @@ -87,6 +87,10 @@ public static string Dehydrate(CollectObject colObj) return JsonConvert.DeserializeObject(serialized, jsonSettings); case RESULT_TYPE.FILEMONITOR: return JsonConvert.DeserializeObject(serialized, jsonSettings); + + case RESULT_TYPE.LOADPOINT: + return JsonConvert.DeserializeObject(serialized, jsonSettings); + default: return null; } diff --git a/Lib/Utils/PermissionUtils.cs b/Lib/Utils/PermissionUtils.cs new file mode 100644 index 000000000..33c0ed223 --- /dev/null +++ b/Lib/Utils/PermissionUtils.cs @@ -0,0 +1,311 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; +using System.Runtime.Versioning; +using System.Security.AccessControl; +using System.Security.Principal; + +namespace Microsoft.CST.AttackSurfaceAnalyzer.Utils +{ + /// + /// The single definition of "writable by an unprivileged user" used by the load point analysis. + /// + /// + /// + /// A securable object counts as user-writable when a principal that every unprivileged + /// interactive user belongs to is granted a right that permits planting or replacing content: + /// + /// + /// S-1-1-0 Everyone + /// S-1-5-4 NT AUTHORITY\INTERACTIVE + /// S-1-5-11 NT AUTHORITY\Authenticated Users + /// S-1-5-32-545 BUILTIN\Users + /// + /// + /// Deny takes precedence over Allow, per principal. This is the canonical simplification of + /// Windows access evaluation; it ignores ACE ordering, which cannot change the outcome for a + /// canonical ACL. + /// + /// + /// Principals are matched by SID and by translated account name because resolves S-1-5 SIDs to NTAccount form but leaves others (such + /// as Everyone, S-1-1-0) as raw SID strings. + /// + /// + public static class PermissionUtils + { + /// + /// Whether the principal is one every unprivileged interactive user belongs to. + /// + public static bool IsUnprivilegedPrincipal(string? principal) + { + if (string.IsNullOrWhiteSpace(principal)) + { + return false; + } + + var trimmed = principal.Trim(); + + if (UnprivilegedPrincipals.Contains(trimmed)) + { + return true; + } + + // Tolerate a domain-qualified form we do not have an exact entry for. + var separator = trimmed.LastIndexOf('\\'); + return separator >= 0 && UnprivilegedPrincipals.Contains(trimmed.Substring(separator + 1)); + } + + /// + /// Whether a registry right name permits writing to the key. + /// + public static bool IsRegistryWriteRight(string? right) + => right is not null && RegistryWriteRights.Contains(right.Trim()); + + /// + /// Whether a file system right name permits creating or replacing content. + /// + public static bool IsFileWriteRight(string? right) + => right is not null && FileWriteRights.Contains(right.Trim()); + + /// + /// Evaluates a live registry ACL. + /// + [SupportedOSPlatform("windows")] + public static bool IsUserWritable(RegistrySecurity security) + { + if (security is null) + { + throw new ArgumentNullException(nameof(security)); + } + + return Evaluate(security.GetAccessRules(true, true, typeof(SecurityIdentifier)) + .OfType() + .Select(rule => ( + AsaHelpers.SidToName(rule.IdentityReference), + rule.AccessControlType, + SplitRights(rule.RegistryRights.ToString()))), + IsRegistryWriteRight); + } + + /// + /// Evaluates a live file system ACL. + /// + [SupportedOSPlatform("windows")] + public static bool IsUserWritable(FileSystemSecurity security) + { + if (security is null) + { + throw new ArgumentNullException(nameof(security)); + } + + return Evaluate(security.GetAccessRules(true, true, typeof(SecurityIdentifier)) + .OfType() + .Select(rule => ( + AsaHelpers.SidToName(rule.IdentityReference), + rule.AccessControlType, + SplitRights(rule.FileSystemRights.ToString()))), + IsFileWriteRight); + } + + /// + /// Evaluates the permissions stored on a collected , which + /// encode the access control type as an "Allow:Right" / "Deny:Right" prefix. + /// + public static bool IsUserWritable(Dictionary>? permissions) + { + if (permissions is null) + { + return false; + } + + return Evaluate(permissions.SelectMany(entry => entry.Value.Select(right => + { + var (type, name) = SplitAccessControlType(right); + return (entry.Key, type, (IEnumerable)new[] { name }); + })), IsRegistryWriteRight); + } + + /// + /// Evaluates the permissions stored on a collected . + /// + /// + /// The file system collector does not record the access control type, so every entry is treated + /// as an Allow. Prefer the overload where the live ACL is + /// available; it honors Deny. + /// + public static bool IsUserWritable(Dictionary? permissions) + { + if (permissions is null) + { + return false; + } + + return Evaluate(permissions.Select(entry => + (entry.Key, AccessControlType.Allow, SplitRights(entry.Value))), IsFileWriteRight); + } + + /// + /// Walks up from to the closest ancestor that exists on disk. Returns + /// null when nothing along the chain exists. + /// + /// + /// When a load point target is missing, the security-relevant ACL is the one on the directory + /// that would receive the file. + /// + public static string? NearestExistingParent(string? path) + { + if (string.IsNullOrWhiteSpace(path)) + { + return null; + } + + string? current; + + try + { + current = System.IO.Path.GetDirectoryName(System.IO.Path.GetFullPath(path)); + } + catch (Exception) + { + return null; + } + + while (!string.IsNullOrEmpty(current)) + { + try + { + if (System.IO.Directory.Exists(current)) + { + return current; + } + + var parent = System.IO.Path.GetDirectoryName(current); + if (string.Equals(parent, current, StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + current = parent; + } + catch (Exception) + { + return null; + } + } + + return null; + } + + /// + /// Splits a comma-joined combined rights mask, as produced by RegistryRights.ToString() and + /// FileSystemRights.ToString(), into individual right names. + /// + public static IEnumerable SplitRights(string? rights) + => string.IsNullOrEmpty(rights) + ? Array.Empty() + : rights!.Split(',').Select(right => right.Trim()); + + /// + /// Encodes an access control type and a right name into the form stored on a RegistryObject. + /// + public static string EncodeRight(AccessControlType type, string right) => $"{type}:{right.Trim()}"; + + /// + /// Reverses . Entries written before the access control type was + /// recorded have no prefix and are read as Allow, which is what they were assumed to be. + /// + public static (AccessControlType Type, string Right) SplitAccessControlType(string right) + { + if (right is null) + { + throw new ArgumentNullException(nameof(right)); + } + + var separator = right.IndexOf(':'); + if (separator > 0) + { + var prefix = right.Substring(0, separator); + if (Enum.TryParse(prefix, out var type)) + { + return (type, right.Substring(separator + 1).Trim()); + } + } + + return (AccessControlType.Allow, right.Trim()); + } + + private static bool Evaluate( + IEnumerable<(string Principal, AccessControlType Type, IEnumerable Rights)> aces, + Func isWriteRight) + { + HashSet allowed = new(StringComparer.OrdinalIgnoreCase); + HashSet denied = new(StringComparer.OrdinalIgnoreCase); + + foreach (var (principal, type, rights) in aces) + { + if (!IsUnprivilegedPrincipal(principal) || !rights.Any(isWriteRight)) + { + continue; + } + + _ = type == AccessControlType.Deny ? denied.Add(principal) : allowed.Add(principal); + } + + allowed.ExceptWith(denied); + return allowed.Count > 0; + } + + private static readonly HashSet UnprivilegedPrincipals = new(StringComparer.OrdinalIgnoreCase) + { + "S-1-1-0", + "Everyone", + "S-1-5-4", + "NT AUTHORITY\\INTERACTIVE", + "INTERACTIVE", + "S-1-5-11", + "NT AUTHORITY\\Authenticated Users", + "Authenticated Users", + "S-1-5-32-545", + "BUILTIN\\Users", + "Users", + }; + + /// + /// RegistryRights members that allow an attacker to change what a key resolves to. + /// + private static readonly HashSet RegistryWriteRights = new(StringComparer.OrdinalIgnoreCase) + { + "SetValue", + "CreateSubKey", + "CreateLink", + "Delete", + "WriteKey", + "ChangePermissions", + "TakeOwnership", + "FullControl", + }; + + /// + /// FileSystemRights members that allow an attacker to plant or replace a binary. Aliased members + /// that share a value (WriteData/CreateFiles, AppendData/CreateDirectories) are both listed + /// because which name ToString() produces is an implementation detail. + /// + private static readonly HashSet FileWriteRights = new(StringComparer.OrdinalIgnoreCase) + { + "WriteData", + "CreateFiles", + "AppendData", + "CreateDirectories", + "Write", + "Modify", + "Delete", + "DeleteSubdirectoriesAndFiles", + "ChangePermissions", + "TakeOwnership", + "FullControl", + }; + } +} diff --git a/Lib/Utils/RegistryWalker.cs b/Lib/Utils/RegistryWalker.cs index 68d15bae2..0e190f301 100644 --- a/Lib/Utils/RegistryWalker.cs +++ b/Lib/Utils/RegistryWalker.cs @@ -65,9 +65,9 @@ public static class RegistryWalker // RegistryRights.ToString() returns a comma joined combined mask. Split it so // individual rights are matchable, and prefix each with the access control type so // Allow and Deny are distinguishable. - foreach (var right in rule.RegistryRights.ToString().Split(',')) + foreach (var right in PermissionUtils.SplitRights(rule.RegistryRights.ToString())) { - var entry = $"{rule.AccessControlType}:{right.Trim()}"; + var entry = PermissionUtils.EncodeRight(rule.AccessControlType, right); if (!rights.Contains(entry)) { rights.Add(entry); @@ -82,10 +82,62 @@ public static class RegistryWalker } regObj.Values = RegistryObject.GetValues(key); + PopulateReferences(regObj); return regObj; } + /// + /// Cracks file paths and CLSIDs out of the key's values so that analysis rules, which cannot + /// follow a reference from one collected object to another, can interrogate them directly. + /// + private static void PopulateReferences(RegistryObject regObj) + { + if (regObj.Values is null || regObj.Values.Count == 0) + { + return; + } + + HashSet paths = new(StringComparer.OrdinalIgnoreCase); + HashSet clsids = new(StringComparer.OrdinalIgnoreCase); + + foreach (var value in regObj.Values.Values) + { + if (paths.Count >= MaxReferencesPerKey && clsids.Count >= MaxReferencesPerKey) + { + break; + } + + if (paths.Count < MaxReferencesPerKey) + { + foreach (var path in RegistryReferenceParser.ExtractPaths(value)) + { + if (paths.Add(path)) + { + regObj.ReferencedPaths.Add(path); + } + } + } + + if (clsids.Count < MaxReferencesPerKey) + { + foreach (var clsid in RegistryReferenceParser.ExtractClsids(value)) + { + if (clsids.Add(clsid)) + { + regObj.ReferencedClsids.Add(clsid); + } + } + } + } + } + + /// + /// Caps the references retained for a single key so that a pathological key cannot blow up the + /// collected object. + /// + private const int MaxReferencesPerKey = 128; + public static IEnumerable WalkHive(RegistryHive Hive, RegistryView View, string startingKey = "") { if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) diff --git a/Tests/AsaAnalyzerTests.cs b/Tests/AsaAnalyzerTests.cs index 27dbea6fd..1eb288716 100644 --- a/Tests/AsaAnalyzerTests.cs +++ b/Tests/AsaAnalyzerTests.cs @@ -4,6 +4,7 @@ using Microsoft.CST.AttackSurfaceAnalyzer.Utils; using Microsoft.CST.OAT; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Microsoft.Win32; using System.Collections.Generic; using System.Linq; @@ -76,5 +77,128 @@ public void VerifyFileMonitorAsFile() private const string TestPathOne = "TestPath1"; private readonly FileMonitorObject testPathOneObject = new(TestPathOne) { FileSystemObject = new FileSystemObject(TestPathOne) { IsExecutable = true } }; + + /// + /// The CVE-2026-50343 shape: a load point key an unprivileged user can write, pointing through a + /// CLSID at a DLL that does not exist in a directory they can also write. + /// + [TestMethod] + public void VerifyLoadPointRulesFlagLayAndWaitConfiguration() + { + var sourceKey = new RegistryObject( + @"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\InstallService\State", + RegistryView.Registry64); + sourceKey.Permissions.Add("NT AUTHORITY\\INTERACTIVE", new List { "Allow:SetValue", "Allow:CreateSubKey" }); + + var loadPoint = new LoadPointObject("StaticPluginMap", sourceKey) + { + SourceValueName = "StaticPluginMap", + SourceKeyUserWritable = true, + TargetClsid = "{E9F83CF2-E0C0-4CA7-AF01-E90C70BEF496}", + TargetPath = @"C:\ProgramData\CrossDevice\CrossDevice.Streaming.Source.dll", + TargetExists = false, + TargetUserWritable = true, + TargetAclSource = "NearestExistingParent", + NearestExistingParentPath = @"C:\ProgramData", + }; + + var matched = AnalyzeLoadPoint(loadPoint); + + Assert.IsTrue(matched.Contains("Load Point Writable by Unprivileged Users")); + Assert.IsTrue(matched.Contains("Load Point Target Missing from Unprivileged Writable Directory")); + Assert.IsTrue(matched.Contains("Privilege Escalation via Unprivileged Load Point")); + + // The DLL does not exist, so the rule about replacing an existing binary must not claim it. + Assert.IsFalse(matched.Contains("Load Point Target Writable by Unprivileged Users")); + } + + /// + /// A correctly ACLed COM registration must not flag. A rule that fires on every COM object is + /// worthless. + /// + [TestMethod] + public void VerifyLoadPointRulesDoNotFlagBenignComRegistration() + { + var sourceKey = new RegistryObject( + @"HKEY_LOCAL_MACHINE\SOFTWARE\Classes\CLSID\{20D04FE0-3AEA-1069-A2D8-08002B30309D}\InprocServer32", + RegistryView.Registry64); + sourceKey.Permissions.Add("NT AUTHORITY\\SYSTEM", new List { "Allow:FullControl" }); + sourceKey.Permissions.Add("BUILTIN\\Administrators", new List { "Allow:FullControl" }); + sourceKey.Permissions.Add("BUILTIN\\Users", new List { "Allow:ReadKey", "Allow:QueryValues" }); + + var loadPoint = new LoadPointObject("ComServer", sourceKey) + { + SourceValueName = string.Empty, + SourceKeyUserWritable = false, + TargetPath = @"C:\Windows\System32\shell32.dll", + TargetExists = true, + TargetUserWritable = false, + TargetAclSource = "Target", + }; + + Assert.AreEqual(0, AnalyzeLoadPoint(loadPoint).Count); + } + + /// + /// A target whose ACL could not be read is not evidence of anything and must not flag. + /// + [TestMethod] + public void VerifyLoadPointRulesDoNotFlagUnreadableTargetAcl() + { + var sourceKey = new RegistryObject(@"HKEY_LOCAL_MACHINE\SOFTWARE\Classes\CLSID\{1}\InprocServer32", RegistryView.Registry64); + sourceKey.Permissions.Add("NT AUTHORITY\\SYSTEM", new List { "Allow:FullControl" }); + + var loadPoint = new LoadPointObject("ComServer", sourceKey) + { + SourceKeyUserWritable = false, + TargetPath = @"C:\Windows\System32\protected.dll", + TargetExists = true, + TargetUserWritable = false, + TargetAclUnavailable = true, + TargetAclSource = "Target", + }; + + Assert.AreEqual(0, AnalyzeLoadPoint(loadPoint).Count); + } + + /// + /// An existing binary an unprivileged user can replace is flagged, but not by the + /// missing-target rule. + /// + [TestMethod] + public void VerifyLoadPointRulesFlagWritableExistingTarget() + { + var sourceKey = new RegistryObject(@"HKEY_LOCAL_MACHINE\SOFTWARE\Classes\CLSID\{2}\InprocServer32", RegistryView.Registry64); + sourceKey.Permissions.Add("NT AUTHORITY\\SYSTEM", new List { "Allow:FullControl" }); + + var loadPoint = new LoadPointObject("ComServer", sourceKey) + { + SourceKeyUserWritable = false, + TargetPath = @"C:\ProgramData\Contoso\plugin.dll", + TargetExists = true, + TargetUserWritable = true, + TargetAclSource = "Target", + }; + + var matched = AnalyzeLoadPoint(loadPoint); + + Assert.IsTrue(matched.Contains("Load Point Target Writable by Unprivileged Users")); + Assert.IsFalse(matched.Contains("Load Point Target Missing from Unprivileged Writable Directory")); + Assert.IsFalse(matched.Contains("Load Point Writable by Unprivileged Users")); + } + + private static HashSet AnalyzeLoadPoint(LoadPointObject loadPoint) + { + var analyzer = new AsaAnalyzer(); + var rules = RuleFile.LoadEmbeddedFilters().Rules + .Where(rule => rule.ResultType == RESULT_TYPE.LOADPOINT) + .ToList(); + + Assert.IsTrue(rules.Count > 0, "No load point rules are present in the embedded rule file."); + + return analyzer.Analyze(rules, new CompareResult() { Compare = loadPoint }) + .Select(rule => rule.Name) + .ToHashSet(); + } } } \ No newline at end of file diff --git a/Tests/CollectorTests.cs b/Tests/CollectorTests.cs index 207d12d43..10948c229 100644 --- a/Tests/CollectorTests.cs +++ b/Tests/CollectorTests.cs @@ -67,9 +67,49 @@ public void TestComObjectCollector() coc.TryExecute(); Assert.IsTrue(results.Any(x => x is ComObject y && y.x86_Binary != null)); + + // The 64-bit view is parsed too, and no resolved path may still contain an unexpanded + // environment variable. + Assert.IsTrue(results.Any(x => x is ComObject y && y.x64_Binary != null)); + Assert.IsFalse(results.Any(x => x is ComObject y + && ((y.x86_Binary?.Path.Contains('%') ?? false) || (y.x64_Binary?.Path.Contains('%') ?? false)))); } } + /// + /// Requires admin. Load points are Windows only; off Windows only the platform gate is asserted. + /// + [TestMethod] + public void TestLoadPointCollector() + { + var lpc = new LoadPointCollector(new CollectorOptions() { SingleThread = true }); + + Assert.AreEqual(RuntimeInformation.IsOSPlatform(OSPlatform.Windows), lpc.CanRunOnPlatform()); + + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + + ConcurrentStack results = new(); + lpc = new LoadPointCollector(new CollectorOptions() { SingleThread = true }, x => results.Push(x)); + lpc.TryExecute(); + + Assert.IsTrue(results.Any(x => x is LoadPointObject y && !string.IsNullOrEmpty(y.TargetPath))); + + // Services are one of the default definitions and always resolve to something on a real system. + Assert.IsTrue(results.Any(x => x is LoadPointObject y && y.LoadPointType == "Service")); + + // Every object must say whether its target exists and where its verdict came from. + Assert.IsTrue(results.OfType().All(y => + y.TargetAclSource is "Target" or "NearestExistingParent" or "None")); + + // System32 binaries must never be reported as writable by unprivileged users. + Assert.IsFalse(results.OfType().Any(y => + y.TargetUserWritable + && (y.TargetPath?.StartsWith(Environment.SystemDirectory, StringComparison.OrdinalIgnoreCase) ?? false))); + } + /// /// Administrator not required /// diff --git a/Tests/HydrationTests.cs b/Tests/HydrationTests.cs index 741334dfd..cf486df2f 100644 --- a/Tests/HydrationTests.cs +++ b/Tests/HydrationTests.cs @@ -116,6 +116,72 @@ public void TestSerializeAndDeserializeRegistryObject() Assert.IsTrue(ro.RowKey.Equals(JsonUtils.Hydrate(JsonUtils.Dehydrate(ro), RESULT_TYPE.REGISTRY)?.RowKey)); } + [TestMethod] + public void TestSerializeAndDeserializeRegistryObjectReferences() + { + var ro = new RegistryObject("Test Key", Microsoft.Win32.RegistryView.Default) + { + PermissionsString = "O:BAG:SYD:(A;;KA;;;IU)", + ReferencedPaths = { @"C:\ProgramData\Contoso\plugin.dll" }, + ReferencedClsids = { "{E9F83CF2-E0C0-4CA7-AF01-E90C70BEF496}" }, + }; + ro.Permissions.Add("NT AUTHORITY\\INTERACTIVE", new System.Collections.Generic.List { "Allow:SetValue" }); + + if (JsonUtils.Hydrate(JsonUtils.Dehydrate(ro), RESULT_TYPE.REGISTRY) is RegistryObject ro2) + { + Assert.AreEqual(ro.RowKey, ro2.RowKey); + Assert.AreEqual(ro.PermissionsString, ro2.PermissionsString); + CollectionAssert.AreEqual(ro.ReferencedPaths, ro2.ReferencedPaths); + CollectionAssert.AreEqual(ro.ReferencedClsids, ro2.ReferencedClsids); + CollectionAssert.AreEqual(ro.Permissions["NT AUTHORITY\\INTERACTIVE"], ro2.Permissions["NT AUTHORITY\\INTERACTIVE"]); + } + else + { + Assert.Fail(); + } + } + + [TestMethod] + public void TestSerializeAndDeserializeLoadPointObject() + { + var sourceKey = new RegistryObject(@"HKEY_LOCAL_MACHINE\SOFTWARE\Test", Microsoft.Win32.RegistryView.Registry64); + sourceKey.Permissions.Add("NT AUTHORITY\\INTERACTIVE", new System.Collections.Generic.List { "Allow:SetValue" }); + + var lp = new LoadPointObject("StaticPluginMap", sourceKey) + { + SourceValueName = "StaticPluginMap", + SourceValueData = "1:{E9F83CF2-E0C0-4CA7-AF01-E90C70BEF496}", + SourceKeyUserWritable = true, + TargetClsid = "{E9F83CF2-E0C0-4CA7-AF01-E90C70BEF496}", + TargetPath = @"C:\ProgramData\CrossDevice\CrossDevice.Streaming.Source.dll", + TargetExists = false, + TargetUserWritable = true, + TargetAclSource = "NearestExistingParent", + NearestExistingParentPath = @"C:\ProgramData", + View = Microsoft.Win32.RegistryView.Registry64, + ResolutionChain = { "step one", "step two" }, + }; + + if (JsonUtils.Hydrate(JsonUtils.Dehydrate(lp), RESULT_TYPE.LOADPOINT) is LoadPointObject lp2) + { + Assert.AreEqual(lp.RowKey, lp2.RowKey); + Assert.AreEqual(lp.Identity, lp2.Identity); + Assert.AreEqual(lp.LoadPointType, lp2.LoadPointType); + Assert.AreEqual(lp.SourceKeyUserWritable, lp2.SourceKeyUserWritable); + Assert.AreEqual(lp.TargetUserWritable, lp2.TargetUserWritable); + Assert.AreEqual(lp.TargetExists, lp2.TargetExists); + Assert.AreEqual(lp.TargetAclSource, lp2.TargetAclSource); + Assert.AreEqual(lp.TargetPath, lp2.TargetPath); + Assert.AreEqual(lp.NearestExistingParentPath, lp2.NearestExistingParentPath); + Assert.AreEqual(lp.SourceKey.Key, lp2.SourceKey.Key); + CollectionAssert.AreEqual(lp.ResolutionChain, lp2.ResolutionChain); + } + else + { + Assert.Fail(); + } + } + [TestMethod] public void TestSerializeAndDeserializeServiceObject() { diff --git a/Tests/LoadPointUtilsTests.cs b/Tests/LoadPointUtilsTests.cs new file mode 100644 index 000000000..3a739420f --- /dev/null +++ b/Tests/LoadPointUtilsTests.cs @@ -0,0 +1,246 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. +using Microsoft.CST.AttackSurfaceAnalyzer.Utils; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; + +namespace Microsoft.CST.AttackSurfaceAnalyzer.Tests +{ + /// + /// Coverage for the pieces of the load point analysis that are platform independent: cracking + /// references out of registry values, and deciding whether a principal can write something. + /// + [TestClass, TestCategory("PipelineSafeTests")] + public class LoadPointUtilsTests + { + [ClassInitialize] + public static void ClassSetup(TestContext _) + { + Logger.Setup(false, true); + Strings.Setup(); + } + + [TestMethod] + public void ExtractPathsFindsDriveQualifiedPaths() + { + var paths = RegistryReferenceParser.ExtractPaths(@"C:\Windows\System32\shell32.dll").ToList(); + + Assert.AreEqual(1, paths.Count); + Assert.AreEqual(@"C:\Windows\System32\shell32.dll", paths[0]); + } + + [TestMethod] + public void ExtractPathsExpandsEnvironmentVariables() + { + Environment.SetEnvironmentVariable(TestVariable, @"C:\ProgramData"); + + try + { + var paths = RegistryReferenceParser + .ExtractPaths($@"%{TestVariable}%\CrossDevice\CrossDevice.Streaming.Source.dll") + .ToList(); + + Assert.AreEqual(1, paths.Count); + Assert.AreEqual(@"C:\ProgramData\CrossDevice\CrossDevice.Streaming.Source.dll", paths[0]); + } + finally + { + Environment.SetEnvironmentVariable(TestVariable, null); + } + } + + [TestMethod] + public void ExtractPathsKeepsSpacesAndTrimsTrailingArguments() + { + var paths = RegistryReferenceParser.ExtractPaths(@"C:\Program Files\Contoso\app.dll,-100").ToList(); + + Assert.AreEqual(1, paths.Count); + Assert.AreEqual(@"C:\Program Files\Contoso\app.dll", paths[0]); + } + + [TestMethod] + public void ExtractPathsDoesNotTruncateLongExtensions() + { + var paths = RegistryReferenceParser.ExtractPaths(@"C:\a.b\c.config").ToList(); + + Assert.AreEqual(1, paths.Count); + Assert.AreEqual(@"C:\a.b\c.config", paths[0]); + } + + [TestMethod] + public void ExtractPathsIgnoresValuesWithoutReferences() + { + Assert.AreEqual(0, RegistryReferenceParser.ExtractPaths("1").Count()); + Assert.AreEqual(0, RegistryReferenceParser.ExtractPaths("SomeDisplayName").Count()); + Assert.AreEqual(0, RegistryReferenceParser.ExtractPaths(null).Count()); + } + + [TestMethod] + public void ExtractPathsSkipsPathologicallyLongValues() + { + var value = new string('A', RegistryReferenceParser.MaxScannedValueLength + 1); + + Assert.AreEqual(0, RegistryReferenceParser.ExtractPaths(value).Count()); + Assert.AreEqual(0, RegistryReferenceParser.ExtractClsids(value).Count()); + } + + [TestMethod] + public void ExtractClsidsNormalizesToBracedUppercase() + { + var clsids = RegistryReferenceParser + .ExtractClsids("PluginId=1;Clsid=e9f83cf2-e0c0-4ca7-af01-e90c70bef496") + .ToList(); + + Assert.AreEqual(1, clsids.Count); + Assert.AreEqual("{E9F83CF2-E0C0-4CA7-AF01-E90C70BEF496}", clsids[0]); + } + + [TestMethod] + public void ExtractClsidsDeduplicates() + { + var clsids = RegistryReferenceParser + .ExtractClsids("{E9F83CF2-E0C0-4CA7-AF01-E90C70BEF496} {e9f83cf2-e0c0-4ca7-af01-e90c70bef496}") + .ToList(); + + Assert.AreEqual(1, clsids.Count); + } + + [TestMethod] + public void ExtractExecutablePathHandlesQuotedCommandLines() + { + Assert.AreEqual( + @"C:\Program Files\Contoso\server.exe", + RegistryReferenceParser.ExtractExecutablePath(@"""C:\Program Files\Contoso\server.exe"" -Embedding")); + } + + [TestMethod] + public void ExtractExecutablePathHandlesUnquotedCommandLines() + { + Assert.AreEqual( + @"C:\Windows\System32\svchost.exe", + RegistryReferenceParser.ExtractExecutablePath(@"C:\Windows\System32\svchost.exe -k netsvcs")); + } + + [TestMethod] + public void NormalizePathStripsQuotesAndNativePrefixes() + { + Assert.AreEqual(@"C:\Windows\System32\drivers\x.sys", RegistryReferenceParser.NormalizePath(@"\??\C:\Windows\System32\drivers\x.sys")); + Assert.AreEqual(@"C:\Windows\System32\x.dll", RegistryReferenceParser.NormalizePath(@" ""C:\Windows\System32\x.dll"" ")); + } + + [TestMethod] + public void NormalizePathQualifiesBareBinaryNames() + { + // Environment.SystemDirectory is empty off Windows, so only the file name is asserted here. + var normalized = RegistryReferenceParser.NormalizePath("shell32.dll"); + + Assert.IsNotNull(normalized); + Assert.AreEqual("shell32.dll", Path.GetFileName(normalized)); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.IsTrue(normalized!.StartsWith(Environment.SystemDirectory, StringComparison.OrdinalIgnoreCase)); + } + } + + [TestMethod] + public void RegistryPermissionsAreUserWritableWhenInteractiveMaySetValue() + { + var permissions = new Dictionary> + { + { "NT AUTHORITY\\SYSTEM", new List { "Allow:FullControl" } }, + { "NT AUTHORITY\\INTERACTIVE", new List { "Allow:SetValue", "Allow:CreateSubKey" } }, + }; + + Assert.IsTrue(PermissionUtils.IsUserWritable(permissions)); + } + + [TestMethod] + public void RegistryPermissionsHonorDenyAces() + { + var permissions = new Dictionary> + { + { "NT AUTHORITY\\INTERACTIVE", new List { "Allow:SetValue", "Deny:SetValue" } }, + }; + + Assert.IsFalse(PermissionUtils.IsUserWritable(permissions)); + } + + [TestMethod] + public void RegistryPermissionsIgnorePrivilegedPrincipalsAndReadRights() + { + Assert.IsFalse(PermissionUtils.IsUserWritable(new Dictionary> + { + { "BUILTIN\\Administrators", new List { "Allow:FullControl" } }, + { "NT AUTHORITY\\SYSTEM", new List { "Allow:FullControl" } }, + { "BUILTIN\\Users", new List { "Allow:ReadKey", "Allow:QueryValues" } }, + })); + } + + [TestMethod] + public void RegistryPermissionsWithoutAccessTypePrefixAreReadAsAllow() + { + // Databases collected before the access control type was recorded have unprefixed rights. + Assert.IsTrue(PermissionUtils.IsUserWritable(new Dictionary> + { + { "BUILTIN\\Users", new List { "WriteKey" } }, + })); + } + + [TestMethod] + public void EveryoneIsRecognizedByRawSidAndByName() + { + Assert.IsTrue(PermissionUtils.IsUnprivilegedPrincipal("S-1-1-0")); + Assert.IsTrue(PermissionUtils.IsUnprivilegedPrincipal("Everyone")); + Assert.IsTrue(PermissionUtils.IsUnprivilegedPrincipal("NT AUTHORITY\\INTERACTIVE")); + Assert.IsTrue(PermissionUtils.IsUnprivilegedPrincipal("BUILTIN\\Users")); + Assert.IsTrue(PermissionUtils.IsUnprivilegedPrincipal("S-1-5-32-545")); + Assert.IsFalse(PermissionUtils.IsUnprivilegedPrincipal("NT AUTHORITY\\SYSTEM")); + Assert.IsFalse(PermissionUtils.IsUnprivilegedPrincipal("BUILTIN\\Administrators")); + Assert.IsFalse(PermissionUtils.IsUnprivilegedPrincipal(null)); + } + + [TestMethod] + public void FilePermissionsAreUserWritableWhenUsersMayCreateFiles() + { + Assert.IsTrue(PermissionUtils.IsUserWritable(new Dictionary + { + { "BUILTIN\\Users", "CreateFiles,AppendData,ReadAndExecute" }, + })); + + Assert.IsFalse(PermissionUtils.IsUserWritable(new Dictionary + { + { "BUILTIN\\Users", "ReadAndExecute,Synchronize" }, + })); + } + + [TestMethod] + public void NearestExistingParentWalksUpToTheDirectoryThatWouldReceiveTheFile() + { + var root = Directory.CreateTempSubdirectory("asa-loadpoint-").FullName; + + try + { + var missing = Path.Combine(root, "Contoso", "Nested", "planted.dll"); + + Assert.AreEqual(root, PermissionUtils.NearestExistingParent(missing)); + } + finally + { + Directory.Delete(root, true); + } + } + + [TestMethod] + public void NearestExistingParentReturnsNullForEmptyInput() + { + Assert.IsNull(PermissionUtils.NearestExistingParent(null)); + Assert.IsNull(PermissionUtils.NearestExistingParent(" ")); + } + + private const string TestVariable = "ASA_LOADPOINT_TEST_DIR"; + } +} diff --git a/analyses.json b/analyses.json index 2fb3f2076..ccf179805 100644 --- a/analyses.json +++ b/analyses.json @@ -2059,6 +2059,110 @@ ] } ] + }, + { + "Name": "Load Point Writable by Unprivileged Users", + "Description": "The registry key that tells Windows what code to load here can be modified by unprivileged users, who can therefore redirect it at code of their choosing. If the code is then loaded by a privileged process this is a privilege escalation.", + "Flag": "WARNING", + "ResultType": "LOADPOINT", + "Platforms": [ + "WINDOWS" + ], + "Clauses": [ + { + "Field": "SourceKeyUserWritable", + "Operation": "IsTrue" + } + ] + }, + { + "Name": "Load Point Target Writable by Unprivileged Users", + "Description": "The binary this load point resolves to can be replaced by unprivileged users. Any privileged process that loads it will run their code.", + "Flag": "WARNING", + "ResultType": "LOADPOINT", + "Platforms": [ + "WINDOWS" + ], + "Expression": "TARGET_WRITABLE AND TARGET_EXISTS AND NOT ACL_UNAVAILABLE", + "Clauses": [ + { + "Label": "TARGET_WRITABLE", + "Field": "TargetUserWritable", + "Operation": "IsTrue" + }, + { + "Label": "TARGET_EXISTS", + "Field": "TargetExists", + "Operation": "IsTrue" + }, + { + "Label": "ACL_UNAVAILABLE", + "Field": "TargetAclUnavailable", + "Operation": "IsTrue" + } + ] + }, + { + "Name": "Load Point Target Missing from Unprivileged Writable Directory", + "Description": "This load point resolves to a binary that does not exist, in a directory unprivileged users can write to. An attacker can simply create the file and wait for a privileged process to load it.", + "Flag": "WARNING", + "ResultType": "LOADPOINT", + "Platforms": [ + "WINDOWS" + ], + "Expression": "PARENT_WRITABLE AND PARENT_ACL AND NOT TARGET_EXISTS AND NOT ACL_UNAVAILABLE", + "Clauses": [ + { + "Label": "PARENT_WRITABLE", + "Field": "TargetUserWritable", + "Operation": "IsTrue" + }, + { + "Label": "PARENT_ACL", + "Field": "TargetAclSource", + "Operation": "Equals", + "Data": [ + "NearestExistingParent" + ] + }, + { + "Label": "TARGET_EXISTS", + "Field": "TargetExists", + "Operation": "IsTrue" + }, + { + "Label": "ACL_UNAVAILABLE", + "Field": "TargetAclUnavailable", + "Operation": "IsTrue" + } + ] + }, + { + "Name": "Privilege Escalation via Unprivileged Load Point", + "Description": "Both ends of this load point are under unprivileged control: the registry key naming the code can be modified, and the binary it resolves to is writable or absent from a writable directory. This is the shape of a lay-and-wait DLL load privilege escalation.", + "Flag": "WARNING", + "ResultType": "LOADPOINT", + "Platforms": [ + "WINDOWS" + ], + "Expression": "SOURCE_WRITABLE AND TARGET_WRITABLE AND NOT ACL_UNAVAILABLE", + "Clauses": [ + { + "Label": "SOURCE_WRITABLE", + "Field": "SourceKeyUserWritable", + "Operation": "IsTrue" + }, + { + "Label": "TARGET_WRITABLE", + "Field": "TargetUserWritable", + "Operation": "IsTrue" + }, + { + "Label": "ACL_UNAVAILABLE", + "Field": "TargetAclUnavailable", + "Operation": "IsTrue" + } + ] } ], "DefaultLevels": { @@ -2078,6 +2182,7 @@ "KEY": "INFORMATION", "PROCESS": "INFORMATION", "DRIVER": "INFORMATION", - "WIFI": "INFORMATION" + "WIFI": "INFORMATION", + "LOADPOINT": "INFORMATION" } } \ No newline at end of file From 020d19d3598cbd07a7dcca260d7871509884923d Mon Sep 17 00:00:00 2001 From: Giulia Stocco <98900+gfs@users.noreply.github.com> Date: Sat, 1 Aug 2026 15:59:23 -0700 Subject: [PATCH 4/4] Do not resolve load point targets on other machines by default A path read out of the registry does not have to name this machine, and whoever can write the value picks which machine it names. Resolving one was not a passive read: File.Exists, the ACL lookup and FilePathToFileSystemObject each open a session to that host and authenticate as the account running the collection, which for these collectors is usually an administrator, and then read content that host controls. The StaticPluginMap key the load point collector was written for grants INTERACTIVE SetValue, so an unprivileged user could choose the host. Load point targets and COM server binaries that resolve through a UNC path or a mapped network drive are now reported by path and left alone. The path is still recorded, and LoadPointObject.TargetIsNetworkPath says why nothing else was collected, with TargetAclUnavailable set so the existing rules do not read the absence of an ACL as a verdict. --follow-network-paths opts back in, in the same way --download-cloud opts into hydrating cloud placeholders. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d4d92e29-a96d-4ec9-91f7-fe7d4a350534 --- .../LoadPointCollectorOptions.razor | 4 + Lib/Collectors/ComObjectCollector.cs | 26 ++++-- Lib/Collectors/LoadPointCollector.cs | 19 ++++- Lib/Objects/CommandOptions.cs | 4 + Lib/Objects/LoadPointObject.cs | 8 ++ Lib/Utils/PathUtils.cs | 85 +++++++++++++++++++ Tests/CollectorTests.cs | 55 ++++++++++++ Tests/HydrationTests.cs | 2 + Tests/LoadPointUtilsTests.cs | 20 +++++ 9 files changed, 215 insertions(+), 8 deletions(-) create mode 100644 Lib/Utils/PathUtils.cs diff --git a/Cli/Components/CollectorOptions/LoadPointCollectorOptions.razor b/Cli/Components/CollectorOptions/LoadPointCollectorOptions.razor index 0a820c8c2..6b864f83a 100644 --- a/Cli/Components/CollectorOptions/LoadPointCollectorOptions.razor +++ b/Cli/Components/CollectorOptions/LoadPointCollectorOptions.razor @@ -6,5 +6,9 @@
+
+ + +
diff --git a/Lib/Collectors/ComObjectCollector.cs b/Lib/Collectors/ComObjectCollector.cs index fd3ba9b47..7a1e19cd5 100644 --- a/Lib/Collectors/ComObjectCollector.cs +++ b/Lib/Collectors/ComObjectCollector.cs @@ -26,7 +26,12 @@ public ComObjectCollector(CollectorOptions? opts = null, Action? /// /// The Registry Key to search /// The View of the registry to use - public static IEnumerable ParseComObjects(RegistryKey SearchKey, RegistryView View, bool SingleThreaded = false) + /// Whether to parse subkeys serially + /// + /// Whether to collect metadata for servers that resolve to another machine. Off by default, since + /// reaching one connects to a host named by whoever could write the CLSID. + /// + public static IEnumerable ParseComObjects(RegistryKey SearchKey, RegistryView View, bool SingleThreaded = false, bool FollowNetworkPaths = false) { if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { return new List(); } if (SearchKey == null) { return new List(); } @@ -45,7 +50,7 @@ public static IEnumerable ParseComObjects(RegistryKey SearchKey, if (RegObj != null) { ComObject comObject = new(RegObj); - var binary = ResolveServerBinary(CurrentKey, View, fsc); + var binary = ResolveServerBinary(CurrentKey, View, fsc, FollowNetworkPaths); if (binary is not null) { @@ -122,7 +127,12 @@ public override bool CanRunOnPlatform() /// Reads the default value of the first server subkey present under a CLSID and resolves it to a /// file on disk. /// - private static FileSystemObject? ResolveServerBinary(RegistryKey clsidKey, RegistryView view, FileSystemCollector fsc) + /// + /// A server registered on another machine is reported by path only unless is set. Collecting its metadata would connect to a host named by + /// whoever could write the CLSID, as the account running the collection. + /// + private static FileSystemObject? ResolveServerBinary(RegistryKey clsidKey, RegistryView view, FileSystemCollector fsc, bool followNetworkPaths) { string[] subKeyNames; @@ -165,6 +175,12 @@ public override bool CanRunOnPlatform() if (path is not null) { + if (!followNetworkPaths && PathUtils.IsNetworkPath(path)) + { + Log.Verbose("Not resolving network COM server path {0} for {1}. Pass --follow-network-paths to include it.", path, clsidKey.Name); + return new FileSystemObject(path); + } + return fsc.FilePathToFileSystemObject(path); } } @@ -198,7 +214,7 @@ internal void ParseView(RegistryView view, CancellationToken cancellationToken) var CLSIDs = SearchKey.OpenSubKey("SOFTWARE\\Classes\\CLSID"); if (CLSIDs is not null) { - foreach (var comObj in ParseComObjects(CLSIDs, view, opts.SingleThread)) + foreach (var comObj in ParseComObjects(CLSIDs, view, opts.SingleThread, opts.FollowNetworkPaths)) { if (cancellationToken.IsCancellationRequested) { return; } HandleChange(comObj); @@ -227,7 +243,7 @@ e is ArgumentException using var ComKey = SearchKey.OpenSubKey(subkeyName)?.OpenSubKey("CLSID"); if (ComKey is not null) { - foreach (var comObj in ParseComObjects(ComKey, view, opts.SingleThread)) + foreach (var comObj in ParseComObjects(ComKey, view, opts.SingleThread, opts.FollowNetworkPaths)) { HandleChange(comObj); } diff --git a/Lib/Collectors/LoadPointCollector.cs b/Lib/Collectors/LoadPointCollector.cs index 05677a843..9b1b5e327 100644 --- a/Lib/Collectors/LoadPointCollector.cs +++ b/Lib/Collectors/LoadPointCollector.cs @@ -234,7 +234,7 @@ public IEnumerable ParseDefinition(LoadPointDefinition definiti } [SupportedOSPlatform("windows")] - private static IEnumerable ParseUnit(LoadPointDefinition definition, RegistryKey unit, RegistryView view, RegistryKey? hiveRoot, FileSystemCollector fsc) + private IEnumerable ParseUnit(LoadPointDefinition definition, RegistryKey unit, RegistryView view, RegistryKey? hiveRoot, FileSystemCollector fsc) { List results = new(); @@ -298,7 +298,7 @@ private static IEnumerable> SelectValues(Dictionary } [SupportedOSPlatform("windows")] - private static IEnumerable ResolveReferences( + private IEnumerable ResolveReferences( LoadPointDefinition definition, LoadPointValueSource source, RegistryObject sourceObj, @@ -442,7 +442,7 @@ private static LoadPointObject NewLoadPoint(LoadPointDefinition definition, Regi /// exploitable shape this collector exists to find. /// [SupportedOSPlatform("windows")] - private static void PopulateTarget(LoadPointObject loadPoint, string? path, FileSystemCollector fsc) + private void PopulateTarget(LoadPointObject loadPoint, string? path, FileSystemCollector fsc) { loadPoint.TargetPath = path; @@ -452,6 +452,19 @@ private static void PopulateTarget(LoadPointObject loadPoint, string? path, File return; } + loadPoint.TargetIsNetworkPath = PathUtils.IsNetworkPath(path); + + // A load point can name a share on another machine, and whoever can write the value picks which + // machine. Resolving it would connect to that host as the account running the collection, so the + // path is recorded and left alone unless it was asked for. + if (loadPoint.TargetIsNetworkPath && !opts.FollowNetworkPaths) + { + Log.Verbose("Not resolving network load point target {0}. Pass --follow-network-paths to include it.", path); + loadPoint.TargetAclSource = "None"; + loadPoint.TargetAclUnavailable = true; + return; + } + try { loadPoint.TargetExists = File.Exists(path) || Directory.Exists(path); diff --git a/Lib/Objects/CommandOptions.cs b/Lib/Objects/CommandOptions.cs index ede82d485..e2fbf1f7d 100644 --- a/Lib/Objects/CommandOptions.cs +++ b/Lib/Objects/CommandOptions.cs @@ -40,6 +40,7 @@ public static CollectCommandOptions FromCollectorOptions(CollectorOptions opts) EnableTpmCollector = opts.EnableTpmCollector, EnableUserCollector = opts.EnableUserCollector, EnableWifiCollector = opts.EnableWifiCollector, + FollowNetworkPaths = opts.FollowNetworkPaths, GatherHashes = opts.GatherHashes, GatherVerboseLogs = opts.GatherVerboseLogs, GatherWifiPasswords = opts.GatherWifiPasswords, @@ -110,6 +111,9 @@ public class CollectorOptions : CommandOptions [Option('w', "wifi", Required = false, HelpText = "Enable the saved Wifi information collector")] public bool EnableWifiCollector { get; set; } + [Option("follow-network-paths", Required = false, HelpText = "Resolve load point and COM server paths that name network locations (UNC paths or mapped network drives). Off by default: reaching one connects to a host named by whoever could write the registry value and authenticates as the account running the collection.")] + public bool FollowNetworkPaths { get; set; } + [Option('h', "gather-hashes", Required = false, HelpText = "Hashes every file when using the File Collector. May dramatically increase run time of the scan.")] public bool GatherHashes { get; set; } diff --git a/Lib/Objects/LoadPointObject.cs b/Lib/Objects/LoadPointObject.cs index 8a86359e5..68bd07220 100644 --- a/Lib/Objects/LoadPointObject.cs +++ b/Lib/Objects/LoadPointObject.cs @@ -105,6 +105,14 @@ public LoadPointObject(string LoadPointType, RegistryObject SourceKey) /// public string TargetAclSource { get; set; } = "None"; + /// + /// True when the target names a location on another machine, either a UNC path or a path through + /// a mapped network drive. The target is reported but is not resolved unless collection was asked + /// to follow network paths, because reaching it connects to a host named by whoever could write + /// the source key and authenticates as the account running the collection. + /// + public bool TargetIsNetworkPath { get; set; } + /// /// True when an unprivileged user can write the target binary, or when the target is missing and /// an unprivileged user can create it in the nearest existing parent directory. diff --git a/Lib/Utils/PathUtils.cs b/Lib/Utils/PathUtils.cs new file mode 100644 index 000000000..bbdbc328a --- /dev/null +++ b/Lib/Utils/PathUtils.cs @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. +using Serilog; +using System; +using System.IO; +using System.Runtime.InteropServices; + +namespace Microsoft.CST.AttackSurfaceAnalyzer.Utils +{ + /// + /// Decides whether a path names something on this machine. + /// + /// + /// + /// Paths recovered from the registry are not necessarily local, and whoever can write the value + /// chooses which machine they name. Resolving one is not a passive read: it opens a session to + /// that host and authenticates as the account running the collection, which for most of these + /// collectors is an administrator, and then reads content that host controls. That is a surprise + /// for anyone who asked only for a local snapshot. + /// + /// + /// Collectors therefore ask this before they touch a path, in the same way the file system + /// collector asks whether a file is a cloud placeholder before hydrating it. + /// + /// + public static class PathUtils + { + /// + /// Whether resolving this path would reach off the machine, through either a UNC path or a drive + /// letter mapped to a network share. Answered from the path itself and the local mount table, so + /// asking never touches the network. + /// + public static bool IsNetworkPath(string? path) + { + if (string.IsNullOrWhiteSpace(path)) + { + return false; + } + + var candidate = path!.Trim(); + + // \\?\ and \\.\ turn off path parsing; what follows the prefix is what is named. \\?\UNC\server\share + // is the extended-length spelling of \\server\share. + if (candidate.StartsWith(@"\\?\", StringComparison.Ordinal) + || candidate.StartsWith(@"\\.\", StringComparison.Ordinal)) + { + candidate = candidate.Substring(4); + + if (candidate.StartsWith(@"UNC\", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + else if (candidate.StartsWith(@"\\", StringComparison.Ordinal)) + { + return true; + } + + return IsNetworkDrive(candidate); + } + + /// + /// Whether a drive-letter rooted path resolves through a mapped network drive. A UNC path reached + /// this way is indistinguishable from a local one by inspection, so the drive itself is asked. + /// + private static bool IsNetworkDrive(string path) + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || path.Length < 2 || path[1] != ':') + { + return false; + } + + try + { + return new DriveInfo(path.Substring(0, 1)).DriveType == DriveType.Network; + } + catch (Exception e) + { + // An unusable drive letter is not evidence that the path is remote. The caller's own error + // handling deals with it when the path fails to resolve. + Log.Verbose("Failed to determine the drive type of {0} ({1}:{2})", path, e.GetType(), e.Message); + return false; + } + } + } +} diff --git a/Tests/CollectorTests.cs b/Tests/CollectorTests.cs index 10948c229..26699219d 100644 --- a/Tests/CollectorTests.cs +++ b/Tests/CollectorTests.cs @@ -108,6 +108,61 @@ public void TestLoadPointCollector() Assert.IsFalse(results.OfType().Any(y => y.TargetUserWritable && (y.TargetPath?.StartsWith(Environment.SystemDirectory, StringComparison.OrdinalIgnoreCase) ?? false))); + + // Nothing off this machine may be touched without being asked for. + Assert.IsFalse(results.OfType().Any(y => y.TargetIsNetworkPath && y.Target is not null)); + } + + /// + /// A load point naming a share on another machine is reported, but resolving it would connect to a + /// host chosen by whoever could write the key, so it is left alone by default. Does not require + /// administrator, and does not touch the network. + /// + [TestMethod] + public void TestLoadPointCollectorDoesNotFollowNetworkPaths() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + + const string NetworkTarget = @"\\asa-test-invalid-host\share\planted.dll"; + + var name = Guid.NewGuid().ToString(); + var key = Registry.CurrentUser.CreateSubKey(name); + key.SetValue("ServiceDll", NetworkTarget); + key.Close(); + + try + { + var definition = new LoadPointDefinition("Test", RegistryHive.CurrentUser, name, false, + new LoadPointValueSource(null, "ServiceDll", LoadPointTargetKind.Path)); + + var lpc = new LoadPointCollector(new CollectorOptions() { SingleThread = true }, definitions: new[] { definition }); + var loadPoints = lpc.ParseDefinition(definition, RegistryView.Default).ToList(); + + Assert.AreEqual(1, loadPoints.Count); + + var loadPoint = loadPoints[0]; + + // The path is still reported, so a rule can see where the load point points. + Assert.AreEqual(NetworkTarget, loadPoint.TargetPath); + Assert.IsTrue(loadPoint.TargetIsNetworkPath); + + // Nothing about the remote end was read, and the object says so rather than implying the + // target is absent or safe. + Assert.IsNull(loadPoint.Target); + Assert.IsNull(loadPoint.NearestExistingParent); + Assert.IsNull(loadPoint.NearestExistingParentPath); + Assert.IsFalse(loadPoint.TargetExists); + Assert.IsFalse(loadPoint.TargetUserWritable); + Assert.IsTrue(loadPoint.TargetAclUnavailable); + Assert.AreEqual("None", loadPoint.TargetAclSource); + } + finally + { + Registry.CurrentUser.DeleteSubKey(name); + } } /// diff --git a/Tests/HydrationTests.cs b/Tests/HydrationTests.cs index cf486df2f..dfb109b74 100644 --- a/Tests/HydrationTests.cs +++ b/Tests/HydrationTests.cs @@ -158,6 +158,7 @@ public void TestSerializeAndDeserializeLoadPointObject() TargetUserWritable = true, TargetAclSource = "NearestExistingParent", NearestExistingParentPath = @"C:\ProgramData", + TargetIsNetworkPath = false, View = Microsoft.Win32.RegistryView.Registry64, ResolutionChain = { "step one", "step two" }, }; @@ -171,6 +172,7 @@ public void TestSerializeAndDeserializeLoadPointObject() Assert.AreEqual(lp.TargetUserWritable, lp2.TargetUserWritable); Assert.AreEqual(lp.TargetExists, lp2.TargetExists); Assert.AreEqual(lp.TargetAclSource, lp2.TargetAclSource); + Assert.AreEqual(lp.TargetIsNetworkPath, lp2.TargetIsNetworkPath); Assert.AreEqual(lp.TargetPath, lp2.TargetPath); Assert.AreEqual(lp.NearestExistingParentPath, lp2.NearestExistingParentPath); Assert.AreEqual(lp.SourceKey.Key, lp2.SourceKey.Key); diff --git a/Tests/LoadPointUtilsTests.cs b/Tests/LoadPointUtilsTests.cs index 3a739420f..0c742a9db 100644 --- a/Tests/LoadPointUtilsTests.cs +++ b/Tests/LoadPointUtilsTests.cs @@ -146,6 +146,26 @@ public void NormalizePathQualifiesBareBinaryNames() } } + [TestMethod] + public void IsNetworkPathDetectsUncPaths() + { + Assert.IsTrue(PathUtils.IsNetworkPath(@"\\attacker\share\planted.dll")); + Assert.IsTrue(PathUtils.IsNetworkPath(@" \\attacker\share\planted.dll ")); + Assert.IsTrue(PathUtils.IsNetworkPath(@"\\?\UNC\attacker\share\planted.dll")); + Assert.IsTrue(PathUtils.IsNetworkPath(@"\\?\unc\attacker\share\planted.dll")); + } + + [TestMethod] + public void IsNetworkPathAcceptsLocalPaths() + { + Assert.IsFalse(PathUtils.IsNetworkPath(@"C:\Windows\System32\shell32.dll")); + // The extended-length prefix does not by itself mean the path is remote. + Assert.IsFalse(PathUtils.IsNetworkPath(@"\\?\C:\Windows\System32\shell32.dll")); + Assert.IsFalse(PathUtils.IsNetworkPath("shell32.dll")); + Assert.IsFalse(PathUtils.IsNetworkPath(null)); + Assert.IsFalse(PathUtils.IsNetworkPath(" ")); + } + [TestMethod] public void RegistryPermissionsAreUserWritableWhenInteractiveMaySetValue() {