mirror of
https://github.com/mRemoteNG/mRemoteNG.git
synced 2026-02-17 22:11:48 +08:00
Add WQL injection prevention via KB ID sanitization
Co-authored-by: Kvarkas <3611964+Kvarkas@users.noreply.github.com>
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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.
|
||||
/// </summary>
|
||||
/// <param name="kbId">The KB ID to sanitize</param>
|
||||
/// <returns>The sanitized KB ID, or empty string if invalid</returns>
|
||||
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();
|
||||
}
|
||||
}
|
||||
}
|
||||
194
mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs
Normal file
194
mRemoteNGTests/Installer/InstalledWindowsUpdateCheckerTests.cs
Normal file
@@ -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
|
||||
}
|
||||
}
|
||||
@@ -69,6 +69,7 @@
|
||||
</ItemGroup>
|
||||
<ItemGroup>
|
||||
<ProjectReference Include="..\mRemoteNG\mRemoteNG.csproj" />
|
||||
<ProjectReference Include="..\mRemoteNGInstaller\CustomActions\CustomActions.csproj" />
|
||||
</ItemGroup>
|
||||
<ItemGroup>
|
||||
<Compile Update="Properties\Resources.Designer.cs">
|
||||
|
||||
Reference in New Issue
Block a user