From a9b7a66a8eefe33247e7059ec7718cc47e134c63 Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Mon, 8 Jun 2026 15:20:23 +0300 Subject: [PATCH] fix: WMI DBNull crash in drive scan and uninstaller prefix bypass Round 3 (P1 crash/security safety): - FixedDriveService read MediaType/BusType with Convert.ToUInt32(value ?? 0u). WMI returns DBNull.Value (not null) for absent properties, so the ?? never fired and Convert.ToUInt32(DBNull.Value) threw, aborting the whole drive scan on some hardware. Reads now use a ToUInt32Safe helper (null and DBNull -> 0). - UninstallerService.IsUnderTrustedDirectory used a bare StartsWith, letting 'C:\Program Files Evil\x.exe' pass the 'C:\Program Files' check. It now compares on a normalized directory boundary so only real sub-paths qualify. Both methods made internal for direct unit testing (InternalsVisibleTo already present). Tests cover DBNull/null/convertible/unconvertible inputs and the sibling-prefix bypass. --- CHANGELOG.md | 6 +++++ .../FixedDriveServiceTests.cs | 21 ++++++++++++++++++ .../UninstallerServiceTests.cs | 22 +++++++++++++++++++ .../SysManager/Services/FixedDriveService.cs | 19 ++++++++++++++-- .../SysManager/Services/UninstallerService.cs | 11 ++++++++-- SysManager/SysManager/SysManager.csproj | 6 ++--- 6 files changed, 78 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df2cb8d..3bf8b8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [1.20.3] - 2026-06-08 + +### Fixed +- **Drive enumeration no longer crashes on missing WMI properties.** `FixedDriveService` read `MediaType`/`BusType` with `Convert.ToUInt32(value ?? 0u)`, but WMI returns `DBNull.Value` (not null) for absent properties, so `Convert.ToUInt32(DBNull.Value)` threw and aborted the whole scan on some hardware. Reads now go through a `ToUInt32Safe` helper that treats null and `DBNull` as 0. +- **Uninstaller trusted-directory check no longer accepts sibling folders.** `IsUnderTrustedDirectory` used a bare `StartsWith`, so `C:\Program Files Evil\…` passed the `C:\Program Files` check. It now compares on a normalized directory boundary (trailing separator) so only true sub-paths of a trusted directory are accepted. + ## [1.20.2] - 2026-06-08 ### Fixed diff --git a/SysManager/SysManager.Tests/FixedDriveServiceTests.cs b/SysManager/SysManager.Tests/FixedDriveServiceTests.cs index eac26d1..59dc160 100644 --- a/SysManager/SysManager.Tests/FixedDriveServiceTests.cs +++ b/SysManager/SysManager.Tests/FixedDriveServiceTests.cs @@ -76,4 +76,25 @@ public void FixedDrive_WithExpression_CreatesModifiedCopy() Assert.Equal("NVMe", b.BusType); Assert.Equal("", a.MediaType); // original unchanged } + + // Regression: WMI returns DBNull.Value (not null) for absent properties, and + // Convert.ToUInt32(DBNull.Value) throws — which crashed drive enumeration. + [Fact] + public void ToUInt32Safe_DBNull_ReturnsZero() + => Assert.Equal(0u, FixedDriveService.ToUInt32Safe(DBNull.Value)); + + [Fact] + public void ToUInt32Safe_Null_ReturnsZero() + => Assert.Equal(0u, FixedDriveService.ToUInt32Safe(null)); + + [Theory] + [InlineData(4, 4u)] + [InlineData((uint)17, 17u)] + [InlineData("11", 11u)] + public void ToUInt32Safe_ConvertibleValue_ReturnsValue(object input, uint expected) + => Assert.Equal(expected, FixedDriveService.ToUInt32Safe(input)); + + [Fact] + public void ToUInt32Safe_Unconvertible_ReturnsZero() + => Assert.Equal(0u, FixedDriveService.ToUInt32Safe("not-a-number")); } diff --git a/SysManager/SysManager.Tests/UninstallerServiceTests.cs b/SysManager/SysManager.Tests/UninstallerServiceTests.cs index 327474a..11da9d4 100644 --- a/SysManager/SysManager.Tests/UninstallerServiceTests.cs +++ b/SysManager/SysManager.Tests/UninstallerServiceTests.cs @@ -13,6 +13,28 @@ namespace SysManager.Tests; /// public class UninstallerServiceTests { + // ── IsUnderTrustedDirectory (regression: prefix-boundary bypass) ── + + [Fact] + public void IsUnderTrustedDirectory_PathInsideProgramFiles_IsTrusted() + { + var pf = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); + Assert.True(UninstallerService.IsUnderTrustedDirectory(System.IO.Path.Combine(pf, "Vendor", "app.exe"))); + } + + [Fact] + public void IsUnderTrustedDirectory_SiblingWithSharedPrefix_IsNotTrusted() + { + // "C:\Program Files Evil\..." must NOT pass the "C:\Program Files" check. + var pf = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); + var evil = pf + " Evil\\malware.exe"; + Assert.False(UninstallerService.IsUnderTrustedDirectory(evil)); + } + + [Fact] + public void IsUnderTrustedDirectory_UntrustedLocation_IsNotTrusted() + => Assert.False(UninstallerService.IsUnderTrustedDirectory(@"C:\Temp\random\app.exe")); + // ── ParseListTable ── [Fact] diff --git a/SysManager/SysManager/Services/FixedDriveService.cs b/SysManager/SysManager/Services/FixedDriveService.cs index 4cdf019..bb779cc 100644 --- a/SysManager/SysManager/Services/FixedDriveService.cs +++ b/SysManager/SysManager/Services/FixedDriveService.cs @@ -64,8 +64,8 @@ public static IReadOnlyList Enumerate() { var id = mo["DeviceId"]?.ToString() ?? ""; media[id] = ( - MapMedia(Convert.ToUInt32(mo["MediaType"] ?? 0u)), - MapBus(Convert.ToUInt32(mo["BusType"] ?? 0u))); + MapMedia(ToUInt32Safe(mo["MediaType"])), + MapBus(ToUInt32Safe(mo["BusType"]))); } } @@ -114,6 +114,21 @@ public static IReadOnlyList Enumerate() return drives; } + /// + /// Converts a WMI property value to uint, treating both null and + /// as 0. WMI returns DBNull.Value (not null) for + /// absent properties, and Convert.ToUInt32(DBNull.Value) throws — + /// which previously crashed drive enumeration on common hardware. + /// + internal static uint ToUInt32Safe(object? value) + { + if (value is null || value is DBNull) return 0u; + try { return Convert.ToUInt32(value); } + catch (InvalidCastException) { return 0u; } + catch (FormatException) { return 0u; } + catch (OverflowException) { return 0u; } + } + private static string MapMedia(uint v) => v switch { 3 => "HDD", diff --git a/SysManager/SysManager/Services/UninstallerService.cs b/SysManager/SysManager/Services/UninstallerService.cs index 5e61cfc..982dbea 100644 --- a/SysManager/SysManager/Services/UninstallerService.cs +++ b/SysManager/SysManager/Services/UninstallerService.cs @@ -364,7 +364,7 @@ internal static (string Exe, string Args) ParseUninstallCommand(string command) /// Checks whether the given absolute path resides under a trusted system directory /// (Program Files, Windows, ProgramData, or LocalApplicationData). /// - private static bool IsUnderTrustedDirectory(string fullPath) + internal static bool IsUnderTrustedDirectory(string fullPath) { var trustedDirs = new[] { @@ -375,8 +375,15 @@ private static bool IsUnderTrustedDirectory(string fullPath) Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData) }; + // Compare on a directory boundary, not a raw prefix. A bare StartsWith lets + // "C:\Program Files Evil\x.exe" pass the "C:\Program Files" check — so append + // a trailing separator to both sides before comparing. + static string WithSep(string p) => + p.EndsWith(System.IO.Path.DirectorySeparatorChar) ? p : p + System.IO.Path.DirectorySeparatorChar; + + var candidate = WithSep(System.IO.Path.GetFullPath(fullPath)); return trustedDirs.Any(dir => !string.IsNullOrEmpty(dir) && - fullPath.StartsWith(dir, StringComparison.OrdinalIgnoreCase)); + candidate.StartsWith(WithSep(System.IO.Path.GetFullPath(dir)), StringComparison.OrdinalIgnoreCase)); } } diff --git a/SysManager/SysManager/SysManager.csproj b/SysManager/SysManager/SysManager.csproj index 2cec3e0..055b0cb 100644 --- a/SysManager/SysManager/SysManager.csproj +++ b/SysManager/SysManager/SysManager.csproj @@ -10,9 +10,9 @@ SysManager true NU1603;NU1701 - 1.20.2 - 1.20.2.0 - 1.20.2.0 + 1.20.3 + 1.20.3.0 + 1.20.3.0 SysManager SysManager — Windows system monitoring toolkit by laurentiu021. Network, updates, health, logs, safe deep cleanup. https://github.com/laurentiu021/SystemManager