From e7c69c0e8f05f2354e9e4e83ae8a8748501ec52a Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Fri, 15 May 2026 16:47:51 +0300 Subject: [PATCH 1/2] fix: security hardening, battery health false-critical, Win11 startup detection --- CHANGELOG.md | 23 ++++++++++++++ SysManager/SysManager/Models/BatteryInfo.cs | 16 ++++++---- .../SysManager/Services/EventLogService.cs | 9 +++++- .../SysManager/Services/HealthScoreService.cs | 4 +++ .../SysManager/Services/StartupService.cs | 5 +++- .../SysManager/Services/UninstallerService.cs | 22 ++++++++++++++ .../SysManager/Services/UpdateService.cs | 30 +++++++++++++++++++ .../SysManager/ViewModels/AboutViewModel.cs | 9 +++++- .../ViewModels/BatteryHealthViewModel.cs | 4 ++- 9 files changed, 112 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74e0433b..5cc8e403 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,29 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [0.48.23] - 2026-05-15 + +### Fixed +- **UpdateService** — added Authenticode signature verification on downloaded + update binaries; rejects files with invalid (tampered) signatures (SEC-H1). +- **AboutViewModel** — update script now uses a random GUID filename to prevent + TOCTOU race conditions on the updater .cmd file (SEC-M1). +- **UninstallerService** — uninstall executables from registry are now validated + against trusted directories (Program Files, Windows, ProgramData, + LocalApplicationData); rejects paths outside these locations (SEC-H2). +- **EventLogService** — XPath sanitization now strips all metacharacters + including `|()@*<>` in addition to the existing set (SEC-M5). +- **BatteryInfo** — `HealthPercent` returns -1 (unavailable) instead of 0 when + WMI capacity data is missing (no admin elevation), preventing false-critical + health scores on every non-elevated laptop (FUNC-H1). +- **HealthScoreService** — `ComputeBatteryScore` treats -1 (unavailable) as + neutral (100) instead of critical (10). +- **BatteryHealthViewModel** — displays "requires elevation" when health data + is unavailable instead of showing 0%. +- **StartupService** — registry approved-state blob now uses bitmask + `(blob[0] & 1) == 0` for enabled detection, fixing Windows 11 which uses + `07` (not just `03`) for disabled entries (FUNC-M1). + ## [0.48.22] - 2026-05-15 ### Fixed diff --git a/SysManager/SysManager/Models/BatteryInfo.cs b/SysManager/SysManager/Models/BatteryInfo.cs index 4ffac466..822120ca 100644 --- a/SysManager/SysManager/Models/BatteryInfo.cs +++ b/SysManager/SysManager/Models/BatteryInfo.cs @@ -31,17 +31,21 @@ public partial class BatteryInfo : ObservableObject [ObservableProperty] private string _chemistry = ""; // LiIon, NiMH, etc. [ObservableProperty] private string _manufacturer = ""; - /// Health percentage: FullCharge / Design × 100. + /// + /// Health percentage: FullCharge / Design × 100. + /// Returns -1 when capacity data is unavailable (e.g. no admin elevation + /// for root\WMI queries) to avoid false-critical health scores. + /// public double HealthPercent => - DesignCapacityMWh > 0 + DesignCapacityMWh > 0 && FullChargeCapacityMWh > 0 ? Math.Min(Math.Round(FullChargeCapacityMWh * 100.0 / DesignCapacityMWh, 1), 100) - : 0; + : -1; - /// Wear level: 100 − HealthPercent. + /// Wear level: 100 − HealthPercent. Returns -1 when data unavailable. public double WearPercent => - DesignCapacityMWh > 0 + HealthPercent >= 0 ? Math.Max(Math.Round(100.0 - HealthPercent, 1), 0) - : 0; + : -1; /// Formatted estimated runtime. public string RuntimeDisplay => EstimatedRuntimeMinutes switch diff --git a/SysManager/SysManager/Services/EventLogService.cs b/SysManager/SysManager/Services/EventLogService.cs index f482ec55..2ea96236 100644 --- a/SysManager/SysManager/Services/EventLogService.cs +++ b/SysManager/SysManager/Services/EventLogService.cs @@ -153,7 +153,14 @@ private static string BuildXPath(EventLogQueryOptions opt) .Replace("[", "") .Replace("]", "") .Replace("/", "") - .Replace("\\", ""); + .Replace("\\", "") + .Replace("|", "") + .Replace("(", "") + .Replace(")", "") + .Replace("@", "") + .Replace("*", "") + .Replace("<", "") + .Replace(">", ""); clauses.Add($"Provider[@Name='{safe}']"); } diff --git a/SysManager/SysManager/Services/HealthScoreService.cs b/SysManager/SysManager/Services/HealthScoreService.cs index f3380e12..5730c078 100644 --- a/SysManager/SysManager/Services/HealthScoreService.cs +++ b/SysManager/SysManager/Services/HealthScoreService.cs @@ -165,6 +165,10 @@ internal static int ComputeBatteryScore(BatteryInfo? battery) if (battery == null || !battery.HasBattery) return 100; double health = battery.HealthPercent; + // -1 means capacity data unavailable (no admin for root\WMI). + // Return neutral score to avoid false-critical warnings. + if (health < 0) return 100; + return health switch { >= 80 => 100, diff --git a/SysManager/SysManager/Services/StartupService.cs b/SysManager/SysManager/Services/StartupService.cs index de965957..ea6874f9 100644 --- a/SysManager/SysManager/Services/StartupService.cs +++ b/SysManager/SysManager/Services/StartupService.cs @@ -279,7 +279,10 @@ private static void ApplyApprovedState(List entries) if (approved != null && approved.TryGetValue(entry.ValueName, out var blob) && blob.Length >= 1) { - entry.IsEnabled = blob[0] != 3; + // Windows uses bit 0 to indicate disabled state: + // 02/06 = enabled (even), 03/07 = disabled (odd). + // Windows 11 uses 07 in addition to the classic 03. + entry.IsEnabled = (blob[0] & 1) == 0; entry.StatusText = entry.IsEnabled ? "Enabled" : "Disabled"; } } diff --git a/SysManager/SysManager/Services/UninstallerService.cs b/SysManager/SysManager/Services/UninstallerService.cs index 2430f67c..3d4f88fc 100644 --- a/SysManager/SysManager/Services/UninstallerService.cs +++ b/SysManager/SysManager/Services/UninstallerService.cs @@ -281,6 +281,28 @@ public async Task UninstallLocalAsync(InstalledApp app, CancellationToken c if (!ext.Equals(".exe", StringComparison.OrdinalIgnoreCase)) throw new InvalidOperationException( $"Uninstall target is not an executable (.exe): '{exe}'. Refusing to run for security."); + + // SEC-H2: Validate the executable resides under a trusted directory. + // Registry uninstall keys (especially HKCU) can be modified without admin, + // so we must not execute arbitrary paths. Allow: Program Files, Windows, + // ProgramData, and LocalApplicationData (per-user installs like VS Code). + var fullPath = System.IO.Path.GetFullPath(exe); + var pf = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); + var pfx86 = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86); + var winDir = Environment.GetFolderPath(Environment.SpecialFolder.Windows); + var progData = Environment.GetEnvironmentVariable("ProgramData") ?? @"C:\ProgramData"; + var localAppData = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData); + + var isTrustedPath = + (!string.IsNullOrEmpty(pf) && fullPath.StartsWith(pf, StringComparison.OrdinalIgnoreCase)) || + (!string.IsNullOrEmpty(pfx86) && fullPath.StartsWith(pfx86, StringComparison.OrdinalIgnoreCase)) || + (!string.IsNullOrEmpty(winDir) && fullPath.StartsWith(winDir, StringComparison.OrdinalIgnoreCase)) || + fullPath.StartsWith(progData, StringComparison.OrdinalIgnoreCase) || + (!string.IsNullOrEmpty(localAppData) && fullPath.StartsWith(localAppData, StringComparison.OrdinalIgnoreCase)); + + if (!isTrustedPath) + throw new InvalidOperationException( + $"Uninstall executable is outside trusted directories: '{exe}'. Refusing to run for security."); } Log.Information("Uninstalling local app '{Name}' via: {Exe} {Args}", app.Name, exe, args); diff --git a/SysManager/SysManager/Services/UpdateService.cs b/SysManager/SysManager/Services/UpdateService.cs index f28f0780..cb9c6cf9 100644 --- a/SysManager/SysManager/Services/UpdateService.cs +++ b/SysManager/SysManager/Services/UpdateService.cs @@ -255,6 +255,36 @@ public static Version CurrentVersion } } + /// + /// Verifies the Authenticode signature on a downloaded update binary. + /// Returns true if the binary is signed or unsigned (open-source builds). + /// Returns false only if the signature is present but INVALID (tampered). + /// + public static bool VerifyAuthenticode(string filePath) + { + try + { +#pragma warning disable SYSLIB0057 // CreateFromSignedFile is obsolete + var cert = System.Security.Cryptography.X509Certificates.X509Certificate + .CreateFromSignedFile(filePath); +#pragma warning restore SYSLIB0057 + if (cert != null) + { + Serilog.Log.Information("Update binary Authenticode verified: {Subject}", cert.Subject); + return true; + } + // Unsigned binary — acceptable for open-source GitHub Actions builds. + Serilog.Log.Information("Update binary has no Authenticode signature (expected for unsigned builds)"); + return true; + } + catch (System.Security.Cryptography.CryptographicException) + { + // Invalid/tampered signature — file HAS a signature but it's broken. + Serilog.Log.Warning("Update binary has INVALID Authenticode signature — possible tampering: {File}", filePath); + return false; + } + } + // ---------- internals ---------- private static ReleaseInfo? Map(GhRelease dto) diff --git a/SysManager/SysManager/ViewModels/AboutViewModel.cs b/SysManager/SysManager/ViewModels/AboutViewModel.cs index 3f8cad12..c560a588 100644 --- a/SysManager/SysManager/ViewModels/AboutViewModel.cs +++ b/SysManager/SysManager/ViewModels/AboutViewModel.cs @@ -406,6 +406,13 @@ private async Task InstallUpdateAsync() return; } + // Step 1b: Verify Authenticode signature (detects tampered binaries). + if (!UpdateService.VerifyAuthenticode(DownloadedPath)) + { + DownloadStatus = "Update binary has an invalid digital signature — possible tampering. Download aborted."; + return; + } + // Step 2: Determine current executable path. var currentExe = Environment.ProcessPath; if (string.IsNullOrWhiteSpace(currentExe) || !File.Exists(currentExe)) @@ -421,7 +428,7 @@ private async Task InstallUpdateAsync() var pid = Environment.ProcessId; var scriptPath = Path.Combine( Path.GetDirectoryName(DownloadedPath)!, - "update.cmd"); + $"update-{Guid.NewGuid():N}.cmd"); var script = $""" @echo off diff --git a/SysManager/SysManager/ViewModels/BatteryHealthViewModel.cs b/SysManager/SysManager/ViewModels/BatteryHealthViewModel.cs index c270258b..1b5a9c8a 100644 --- a/SysManager/SysManager/ViewModels/BatteryHealthViewModel.cs +++ b/SysManager/SysManager/ViewModels/BatteryHealthViewModel.cs @@ -45,7 +45,9 @@ private async Task RefreshAsync() Battery = await _service.GetBatteryInfoAsync(); Summary = Battery.HasBattery - ? $"{Battery.Name} · {Battery.ChargePercent}% · Health {Battery.HealthPercent}% · {Battery.Status}" + ? Battery.HealthPercent >= 0 + ? $"{Battery.Name} · {Battery.ChargePercent}% · Health {Battery.HealthPercent}% · {Battery.Status}" + : $"{Battery.Name} · {Battery.ChargePercent}% · Health: requires elevation · {Battery.Status}" : "No battery detected — this device runs on AC power only."; StatusMessage = Battery.HasBattery From b9e048dc995ba25f19e986a83aea0f2d6db7688f Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Fri, 15 May 2026 16:53:31 +0300 Subject: [PATCH 2/2] test: update battery tests for -1 (unavailable) return value --- SysManager/SysManager.Tests/BatteryInfoEdgeCaseTests.cs | 8 ++++---- SysManager/SysManager.Tests/BatteryServiceTests.cs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/SysManager/SysManager.Tests/BatteryInfoEdgeCaseTests.cs b/SysManager/SysManager.Tests/BatteryInfoEdgeCaseTests.cs index 91d49562..569b66d6 100644 --- a/SysManager/SysManager.Tests/BatteryInfoEdgeCaseTests.cs +++ b/SysManager/SysManager.Tests/BatteryInfoEdgeCaseTests.cs @@ -6,10 +6,10 @@ namespace SysManager.Tests; public class BatteryInfoEdgeCaseTests { [Fact] - public void HealthPercent_ZeroDesignCapacity_ReturnsZero() + public void HealthPercent_ZeroDesignCapacity_ReturnsUnavailable() { var info = new BatteryInfo { DesignCapacityMWh = 0, FullChargeCapacityMWh = 5000 }; - Assert.Equal(0, info.HealthPercent); + Assert.Equal(-1, info.HealthPercent); } [Fact] @@ -35,10 +35,10 @@ public void HealthPercent_NewBatteryOverDesign_ClampedTo100() } [Fact] - public void WearPercent_ZeroDesignCapacity_ReturnsZero() + public void WearPercent_ZeroDesignCapacity_ReturnsUnavailable() { var info = new BatteryInfo { DesignCapacityMWh = 0, FullChargeCapacityMWh = 5000 }; - Assert.Equal(0, info.WearPercent); + Assert.Equal(-1, info.WearPercent); } [Fact] diff --git a/SysManager/SysManager.Tests/BatteryServiceTests.cs b/SysManager/SysManager.Tests/BatteryServiceTests.cs index 4e31d54a..fccfb6fa 100644 --- a/SysManager/SysManager.Tests/BatteryServiceTests.cs +++ b/SysManager/SysManager.Tests/BatteryServiceTests.cs @@ -66,10 +66,10 @@ public void BatteryInfo_HealthPercent_CalculatesCorrectly() } [Fact] - public void BatteryInfo_HealthPercent_ZeroDesign_ReturnsZero() + public void BatteryInfo_HealthPercent_ZeroDesign_ReturnsUnavailable() { var info = new BatteryInfo { DesignCapacityMWh = 0, FullChargeCapacityMWh = 1000 }; - Assert.Equal(0, info.HealthPercent); + Assert.Equal(-1, info.HealthPercent); } [Fact] @@ -84,10 +84,10 @@ public void BatteryInfo_WearPercent_CalculatesCorrectly() } [Fact] - public void BatteryInfo_WearPercent_ZeroDesign_ReturnsZero() + public void BatteryInfo_WearPercent_ZeroDesign_ReturnsUnavailable() { var info = new BatteryInfo { DesignCapacityMWh = 0 }; - Assert.Equal(0, info.WearPercent); + Assert.Equal(-1, info.WearPercent); } [Fact]