diff --git a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs index 5fa3d6d2..6df0d6f4 100644 --- a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs +++ b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs @@ -2,12 +2,14 @@ using System.Management; using System.Collections; using System.Collections.Generic; +using System.Text.RegularExpressions; namespace CustomActions { public class InstalledWindowsUpdateChecker { private readonly ManagementScope _managementScope; + private static readonly Regex KbPattern = new Regex(@"^(KB)?\d+$", RegexOptions.IgnoreCase | RegexOptions.Compiled); public InstalledWindowsUpdateChecker() { @@ -61,12 +63,45 @@ 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: optional KB prefix followed by digits + // (e.g., KB1234567 or 1234567) + // Trim whitespace and check if it matches the expected pattern + var trimmedKb = kbId.Trim(); + if (!KbPattern.IsMatch(trimmedKb)) + return string.Empty; + + // Normalize to uppercase + var normalizedKb = trimmedKb.ToUpperInvariant(); + + // Ensure KB prefix is present (Win32_QuickFixEngineering always uses the KB prefix) + if (!normalizedKb.StartsWith("KB")) + normalizedKb = "KB" + normalizedKb; + + return normalizedKb; + } } } \ No newline at end of file diff --git a/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs b/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs new file mode 100644 index 00000000..3be24dfd --- /dev/null +++ b/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs @@ -0,0 +1,208 @@ +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_ReturnsWithKbPrefix() + { + var result = InvokeSanitizeKbId("1234567"); + Assert.That(result, Is.EqualTo("KB1234567")); + } + + [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(Array.Empty()); + 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'")); + } + + [Test] + public void BuildWhereClause_DigitOnlyKb_AddsKbPrefix() + { + var result = InvokeBuildWhereClause(new[] { "1234567" }); + Assert.That(result, Is.EqualTo("HotFixID='KB1234567'")); + } + + [Test] + public void BuildWhereClause_MixedPrefixedAndDigitOnly_NormalizesAll() + { + var result = InvokeBuildWhereClause(new[] { "1234567", "KB7654321", "kb9999999" }); + Assert.That(result, Is.EqualTo("HotFixID='KB1234567' OR HotFixID='KB7654321' OR HotFixID='KB9999999'")); + } + + #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 @@ +