From e9d0a8aa690df0624d2b836f9abe50746419450e Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 30 Dec 2025 12:11:00 +0000
Subject: [PATCH 1/5] Initial plan
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 2/5] 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 @@
+
From 76cb0a1e0bcd78236909007e07437bcb083300e9 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 30 Dec 2025 12:16:52 +0000
Subject: [PATCH 3/5] Address code review feedback - optimize regex and use
Array.Empty
Co-authored-by: Kvarkas <3611964+Kvarkas@users.noreply.github.com>
---
.../CustomActions/InstalledWindowsUpdateChecker.cs | 6 ++----
.../Installer/InstalledWindowsUpdateCheckerTests.cs | 2 +-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs
index 070cbb33..3be331bc 100644
--- a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs
+++ b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs
@@ -9,6 +9,7 @@ namespace CustomActions
public class InstalledWindowsUpdateChecker
{
private readonly ManagementScope _managementScope;
+ private static readonly Regex KbPattern = new Regex(@"^(KB)?\d+$", RegexOptions.IgnoreCase | RegexOptions.Compiled);
public InstalledWindowsUpdateChecker()
{
@@ -88,12 +89,9 @@ namespace CustomActions
// 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))
+ if (!KbPattern.IsMatch(trimmedKb))
return string.Empty;
// Return the sanitized value (uppercased for consistency)
diff --git a/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs b/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs
index f45cc0a6..aa47bc79 100644
--- a/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs
+++ b/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs
@@ -157,7 +157,7 @@ namespace mRemoteNGTests.Installer
[Test]
public void BuildWhereClause_EmptyList_ReturnsEmpty()
{
- var result = InvokeBuildWhereClause(new string[0]);
+ var result = InvokeBuildWhereClause(Array.Empty());
Assert.That(result, Is.Empty);
}
From ff54ca9015466e9c379423d24f218e73a3248f01 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 30 Dec 2025 12:17:52 +0000
Subject: [PATCH 4/5] Fix inaccurate comment in SanitizeKbId method
Co-authored-by: Kvarkas <3611964+Kvarkas@users.noreply.github.com>
---
.../CustomActions/InstalledWindowsUpdateChecker.cs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs
index 3be331bc..48e555be 100644
--- a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs
+++ b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs
@@ -87,8 +87,8 @@ namespace CustomActions
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)
+ // 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))
From dca2517cf04a825f397bb1b894cadb7ed266bd0e Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Tue, 30 Dec 2025 12:19:35 +0000
Subject: [PATCH 5/5] Normalize digit-only KB IDs to include KB prefix
Co-authored-by: Kvarkas <3611964+Kvarkas@users.noreply.github.com>
---
.../InstalledWindowsUpdateChecker.cs | 10 ++++++++--
.../InstalledWindowsUpdateCheckerTests.cs | 18 ++++++++++++++++--
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs
index 48e555be..6df0d6f4 100644
--- a/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs
+++ b/mRemoteNGInstaller/CustomActions/InstalledWindowsUpdateChecker.cs
@@ -94,8 +94,14 @@ namespace CustomActions
if (!KbPattern.IsMatch(trimmedKb))
return string.Empty;
- // Return the sanitized value (uppercased for consistency)
- return trimmedKb.ToUpperInvariant();
+ // 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
index aa47bc79..3be24dfd 100644
--- a/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs
+++ b/mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs
@@ -46,10 +46,10 @@ namespace mRemoteNGTests.Installer
}
[Test]
- public void SanitizeKbId_ValidNumberOnly_ReturnsUppercased()
+ public void SanitizeKbId_ValidNumberOnly_ReturnsWithKbPrefix()
{
var result = InvokeSanitizeKbId("1234567");
- Assert.That(result, Is.EqualTo("1234567"));
+ Assert.That(result, Is.EqualTo("KB1234567"));
}
[Test]
@@ -175,6 +175,20 @@ namespace mRemoteNGTests.Installer
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