From c683854678bdaf769a3cff523f84ecc4d1732715 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Dec 2025 12:15:31 +0000 Subject: [PATCH] Add WQL injection prevention via KB ID sanitization Co-authored-by: Kvarkas <3611964+Kvarkas@users.noreply.github.com> --- .../InstalledWindowsUpdateChecker.cs | 33 ++- .../InstalledWindowsUpdateCheckerTests.cs | 194 ++++++++++++++++++ mRemoteNGTests/mRemoteNGTests.csproj | 1 + 3 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs diff --git a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs index 5fa3d6d2..070cbb33 100644 --- a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs +++ b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs @@ -2,6 +2,7 @@ using System.Management; using System.Collections; using System.Collections.Generic; +using System.Text.RegularExpressions; namespace CustomActions { @@ -61,12 +62,42 @@ namespace CustomActions var counter = 0; foreach (var kb in kbList) { + var sanitizedKb = SanitizeKbId(kb); + if (string.IsNullOrEmpty(sanitizedKb)) + continue; // Skip invalid KB IDs + if (counter > 0) whereClause += " OR "; - whereClause += $"HotFixID='{kb}'"; + whereClause += $"HotFixID='{sanitizedKb}'"; counter++; } return whereClause; } + + /// + /// Sanitizes a KB ID to prevent WQL injection attacks. + /// KB IDs must match the pattern: optional "KB" prefix followed by digits, + /// or just digits. Any other characters are rejected. + /// + /// The KB ID to sanitize + /// The sanitized KB ID, or empty string if invalid + private string SanitizeKbId(string kbId) + { + if (string.IsNullOrWhiteSpace(kbId)) + return string.Empty; + + // KB IDs should match the pattern: KB followed by digits (e.g., KB1234567) + // or just digits (e.g., 1234567) + // This regex allows optional "KB" prefix followed by one or more digits + var kbPattern = new Regex(@"^(KB)?\d+$", RegexOptions.IgnoreCase); + + // Trim whitespace and check if it matches the expected pattern + var trimmedKb = kbId.Trim(); + if (!kbPattern.IsMatch(trimmedKb)) + return string.Empty; + + // Return the sanitized value (uppercased for consistency) + return trimmedKb.ToUpperInvariant(); + } } } \ No newline at end of file diff --git a/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs b/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs new file mode 100644 index 00000000..f45cc0a6 --- /dev/null +++ b/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs @@ -0,0 +1,194 @@ +using NUnit.Framework; +using System; +using System.Reflection; + +namespace mRemoteNGTests.Installer +{ + [TestFixture] + public class InstalledWindowsUpdateCheckerTests + { + private CustomActions.InstalledWindowsUpdateChecker _checker; + private MethodInfo _sanitizeKbIdMethod; + private MethodInfo _buildWhereClauseMethod; + + [SetUp] + public void Setup() + { + _checker = new CustomActions.InstalledWindowsUpdateChecker(); + + // Use reflection to access private methods for testing + var type = typeof(CustomActions.InstalledWindowsUpdateChecker); + _sanitizeKbIdMethod = type.GetMethod("SanitizeKbId", BindingFlags.NonPublic | BindingFlags.Instance); + _buildWhereClauseMethod = type.GetMethod("BuildWhereClauseFromKbList", BindingFlags.NonPublic | BindingFlags.Instance); + } + + #region SanitizeKbId Tests + + [Test] + public void SanitizeKbId_ValidKbWithPrefix_ReturnsUppercased() + { + var result = InvokeSanitizeKbId("KB1234567"); + Assert.That(result, Is.EqualTo("KB1234567")); + } + + [Test] + public void SanitizeKbId_ValidKbLowercase_ReturnsUppercased() + { + var result = InvokeSanitizeKbId("kb1234567"); + Assert.That(result, Is.EqualTo("KB1234567")); + } + + [Test] + public void SanitizeKbId_ValidKbMixedCase_ReturnsUppercased() + { + var result = InvokeSanitizeKbId("Kb1234567"); + Assert.That(result, Is.EqualTo("KB1234567")); + } + + [Test] + public void SanitizeKbId_ValidNumberOnly_ReturnsUppercased() + { + var result = InvokeSanitizeKbId("1234567"); + Assert.That(result, Is.EqualTo("1234567")); + } + + [Test] + public void SanitizeKbId_WithWhitespace_ReturnsTrimmedAndUppercased() + { + var result = InvokeSanitizeKbId(" KB1234567 "); + Assert.That(result, Is.EqualTo("KB1234567")); + } + + [Test] + public void SanitizeKbId_SqlInjectionAttempt_ReturnsEmpty() + { + var result = InvokeSanitizeKbId("KB1234' OR '1'='1"); + Assert.That(result, Is.Empty); + } + + [Test] + public void SanitizeKbId_WqlInjectionWithSemicolon_ReturnsEmpty() + { + var result = InvokeSanitizeKbId("KB1234; DROP TABLE"); + Assert.That(result, Is.Empty); + } + + [Test] + public void SanitizeKbId_WithSpecialCharacters_ReturnsEmpty() + { + var result = InvokeSanitizeKbId("KB1234@#$"); + Assert.That(result, Is.Empty); + } + + [Test] + public void SanitizeKbId_NullInput_ReturnsEmpty() + { + var result = InvokeSanitizeKbId(null); + Assert.That(result, Is.Empty); + } + + [Test] + public void SanitizeKbId_EmptyString_ReturnsEmpty() + { + var result = InvokeSanitizeKbId(""); + Assert.That(result, Is.Empty); + } + + [Test] + public void SanitizeKbId_WhitespaceOnly_ReturnsEmpty() + { + var result = InvokeSanitizeKbId(" "); + Assert.That(result, Is.Empty); + } + + [Test] + public void SanitizeKbId_OnlyKbPrefix_ReturnsEmpty() + { + var result = InvokeSanitizeKbId("KB"); + Assert.That(result, Is.Empty); + } + + [Test] + public void SanitizeKbId_WithDashes_ReturnsEmpty() + { + var result = InvokeSanitizeKbId("KB-1234567"); + Assert.That(result, Is.Empty); + } + + [Test] + public void SanitizeKbId_WithUnderscores_ReturnsEmpty() + { + var result = InvokeSanitizeKbId("KB_1234567"); + Assert.That(result, Is.Empty); + } + + #endregion + + #region BuildWhereClauseFromKbList Tests + + [Test] + public void BuildWhereClause_SingleValidKb_ReturnsCorrectClause() + { + var result = InvokeBuildWhereClause(new[] { "KB1234567" }); + Assert.That(result, Is.EqualTo("HotFixID='KB1234567'")); + } + + [Test] + public void BuildWhereClause_MultipleValidKbs_ReturnsOrClause() + { + var result = InvokeBuildWhereClause(new[] { "KB1234567", "KB7654321" }); + Assert.That(result, Is.EqualTo("HotFixID='KB1234567' OR HotFixID='KB7654321'")); + } + + [Test] + public void BuildWhereClause_InvalidKb_SkipsInvalid() + { + var result = InvokeBuildWhereClause(new[] { "KB1234567", "KB1234'; DROP--", "KB7654321" }); + Assert.That(result, Is.EqualTo("HotFixID='KB1234567' OR HotFixID='KB7654321'")); + } + + [Test] + public void BuildWhereClause_AllInvalidKbs_ReturnsEmpty() + { + var result = InvokeBuildWhereClause(new[] { "'; DROP TABLE", "OR 1=1--" }); + Assert.That(result, Is.Empty); + } + + [Test] + public void BuildWhereClause_EmptyList_ReturnsEmpty() + { + var result = InvokeBuildWhereClause(new string[0]); + Assert.That(result, Is.Empty); + } + + [Test] + public void BuildWhereClause_NullValues_SkipsNulls() + { + var result = InvokeBuildWhereClause(new[] { "KB1234567", null, "KB7654321" }); + Assert.That(result, Is.EqualTo("HotFixID='KB1234567' OR HotFixID='KB7654321'")); + } + + [Test] + public void BuildWhereClause_MixedCaseKbs_NormalizesToUppercase() + { + var result = InvokeBuildWhereClause(new[] { "kb1234567", "KB7654321" }); + Assert.That(result, Is.EqualTo("HotFixID='KB1234567' OR HotFixID='KB7654321'")); + } + + #endregion + + #region Helper Methods + + private string InvokeSanitizeKbId(string input) + { + return (string)_sanitizeKbIdMethod.Invoke(_checker, new object[] { input }); + } + + private string InvokeBuildWhereClause(string[] kbList) + { + return (string)_buildWhereClauseMethod.Invoke(_checker, new object[] { kbList }); + } + + #endregion + } +} diff --git a/mRemoteNGTests/mRemoteNGTests.csproj b/mRemoteNGTests/mRemoteNGTests.csproj index d956a445..d7b7a058 100644 --- a/mRemoteNGTests/mRemoteNGTests.csproj +++ b/mRemoteNGTests/mRemoteNGTests.csproj @@ -69,6 +69,7 @@ +