diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b0b445e..ec50a0f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,17 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [0.48.13] - 2026-05-15 + +### Fixed +- **UninstallerService (SEC-007)** — trusted system binaries (MsiExec, rundll32) + now resolved to absolute System32 path before execution, preventing PATH + hijacking attacks. +- **SpeedTestService (SEC-008)** — Ookla CLI process now killed on timeout or + cancellation, preventing orphan processes consuming resources indefinitely. +- **SpeedTestService (PRIV-001)** — all exception messages in Log.Debug calls + now sanitized via LogService.SanitizePath to prevent username leakage in logs. + ## [0.48.12] - 2026-05-15 ### Fixed diff --git a/SysManager/SysManager/Services/SpeedTestService.cs b/SysManager/SysManager/Services/SpeedTestService.cs index 15802f2d..75198627 100644 --- a/SysManager/SysManager/Services/SpeedTestService.cs +++ b/SysManager/SysManager/Services/SpeedTestService.cs @@ -207,7 +207,17 @@ public async Task RunOoklaAsync( await Task.Run(() => proc.Start(), linked).ConfigureAwait(false); var stdout = await proc.StandardOutput.ReadToEndAsync(linked); var stderr = await proc.StandardError.ReadToEndAsync(linked); - await proc.WaitForExitAsync(linked); + try + { + await proc.WaitForExitAsync(linked); + } + catch (OperationCanceledException) + { + // Timeout or user cancellation — kill the orphan process + try { if (!proc.HasExited) proc.Kill(entireProcessTree: true); } + catch (InvalidOperationException) { /* already exited */ } + throw; + } if (proc.ExitCode != 0) { @@ -215,8 +225,8 @@ public async Task RunOoklaAsync( if (proc.ExitCode == -1073741515) // STATUS_DLL_NOT_FOUND { try { File.Delete(exe); } - catch (IOException ex) { Log.Debug("Cleanup failed (locked): {Error}", ex.Message); } - catch (UnauthorizedAccessException ex) { Log.Debug("Cleanup failed (access): {Error}", ex.Message); } + catch (IOException ex2) { Log.Debug("Cleanup failed (locked): {Error}", LogService.SanitizePath(ex2.Message)); } + catch (UnauthorizedAccessException ex2) { Log.Debug("Cleanup failed (access): {Error}", LogService.SanitizePath(ex2.Message)); } } throw new InvalidOperationException($"Ookla failed ({proc.ExitCode}): {stderr}"); } @@ -276,8 +286,8 @@ private static async Task EnsureOoklaAsync( if (File.Exists(path) && new FileInfo(path).Length < 1024) { try { File.Delete(path); } - catch (IOException ex) { Log.Debug("Cleanup failed (locked): {Error}", ex.Message); } - catch (UnauthorizedAccessException ex) { Log.Debug("Cleanup failed (access): {Error}", ex.Message); } + catch (IOException ex) { Log.Debug("Cleanup failed (locked): {Error}", LogService.SanitizePath(ex.Message)); } + catch (UnauthorizedAccessException ex) { Log.Debug("Cleanup failed (access): {Error}", LogService.SanitizePath(ex.Message)); } } return !File.Exists(path); }, ct).ConfigureAwait(false); @@ -344,8 +354,8 @@ await Task.Run(() => { Log.Warning("Ookla speedtest.exe Authenticode subject mismatch: {Subject}", cert?.Subject ?? "none"); try { File.Delete(exe); } - catch (IOException ex2) { Log.Debug("Cleanup failed (locked): {Error}", ex2.Message); } - catch (UnauthorizedAccessException ex2) { Log.Debug("Cleanup failed (access): {Error}", ex2.Message); } + catch (IOException ex2) { Log.Debug("Cleanup failed (locked): {Error}", LogService.SanitizePath(ex2.Message)); } + catch (UnauthorizedAccessException ex2) { Log.Debug("Cleanup failed (access): {Error}", LogService.SanitizePath(ex2.Message)); } throw new InvalidOperationException( $"Ookla speedtest.exe failed Authenticode verification (subject: {cert?.Subject ?? "none"}). Binary deleted for security."); } @@ -355,8 +365,8 @@ await Task.Run(() => { Log.Warning(ex, "Ookla speedtest.exe has no valid Authenticode signature"); try { File.Delete(exe); } - catch (IOException ex2) { Log.Debug("Cleanup failed (locked): {Error}", ex2.Message); } - catch (UnauthorizedAccessException ex2) { Log.Debug("Cleanup failed (access): {Error}", ex2.Message); } + catch (IOException ex2) { Log.Debug("Cleanup failed (locked): {Error}", LogService.SanitizePath(ex2.Message)); } + catch (UnauthorizedAccessException ex2) { Log.Debug("Cleanup failed (access): {Error}", LogService.SanitizePath(ex2.Message)); } throw new InvalidOperationException( "Ookla speedtest.exe has no valid Authenticode signature. Binary deleted for security.", ex); } diff --git a/SysManager/SysManager/Services/UninstallerService.cs b/SysManager/SysManager/Services/UninstallerService.cs index d05991f9..d69af370 100644 --- a/SysManager/SysManager/Services/UninstallerService.cs +++ b/SysManager/SysManager/Services/UninstallerService.cs @@ -255,6 +255,7 @@ public async Task UninstallLocalAsync(InstalledApp app, CancellationToken c // without admin, so we must not blindly execute whatever is there. // Use exact filename match for system binaries to prevent bypass via // similarly-named executables (e.g. "MsiExecEvil.exe"). + // Resolve trusted binaries to absolute System32 path to prevent PATH hijacking. var exeFileName = System.IO.Path.GetFileName(exe); var isTrustedSystemBinary = exeFileName.Equals("MsiExec.exe", StringComparison.OrdinalIgnoreCase) || @@ -262,7 +263,15 @@ public async Task UninstallLocalAsync(InstalledApp app, CancellationToken c exeFileName.Equals("rundll32.exe", StringComparison.OrdinalIgnoreCase) || exeFileName.Equals("rundll32", StringComparison.OrdinalIgnoreCase); - if (!isTrustedSystemBinary) + if (isTrustedSystemBinary) + { + // Resolve to absolute path in System32 to prevent PATH hijacking + var systemDir = Environment.GetFolderPath(Environment.SpecialFolder.System); + var resolvedName = exeFileName.EndsWith(".exe", StringComparison.OrdinalIgnoreCase) + ? exeFileName : exeFileName + ".exe"; + exe = System.IO.Path.Combine(systemDir, resolvedName); + } + else { if (!System.IO.File.Exists(exe)) throw new InvalidOperationException(