From 01b62bc679684a36bcd42109ae4d54ba499126ed Mon Sep 17 00:00:00 2001 From: xiangyan99 Date: Wed, 18 Mar 2026 15:57:01 -0700 Subject: [PATCH 1/2] MySQL security enhancement --- servers/Azure.Mcp.Server/CHANGELOG.md | 1 + .../src/Services/MySqlService.cs | 77 ++++++++++--------- .../MySqlServiceQueryValidationTests.cs | 41 +++++++++- 3 files changed, 80 insertions(+), 39 deletions(-) diff --git a/servers/Azure.Mcp.Server/CHANGELOG.md b/servers/Azure.Mcp.Server/CHANGELOG.md index 7286ca9bbf..36ab5d1b3f 100644 --- a/servers/Azure.Mcp.Server/CHANGELOG.md +++ b/servers/Azure.Mcp.Server/CHANGELOG.md @@ -11,6 +11,7 @@ The Azure MCP Server updates automatically by default whenever a new release com ### Bugs Fixed - Hardened Postgres SQL query validator to block set-operation keywords (UNION, INTERSECT, EXCEPT), additional dangerous system catalogs, and fixed false-positive comment detection inside string literals +- Fixed SQL injection vulnerability in MySQL query validation that allowed bypassing safety checks via version-specific comments and UNION-based attacks. [[#2083](https://github.com/microsoft/mcp/pull/2083)] ### Other Changes diff --git a/tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs b/tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs index 450e34911c..5e9852abf2 100644 --- a/tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs +++ b/tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs @@ -29,6 +29,8 @@ public class MySqlService(IResourceGroupService resourceGroupService, ITenantSer [ // Data manipulation that could be harmful "DROP", "DELETE", "TRUNCATE", "ALTER", "CREATE", "INSERT", "UPDATE", + // Set operations that can be used for data exfiltration + "UNION", "INTERSECT", "EXCEPT", // Administrative operations "GRANT", "REVOKE", "SET", "RESET", "KILL", "SHUTDOWN", "RESTART", // Information disclosure @@ -59,14 +61,25 @@ public class MySqlService(IResourceGroupService resourceGroupService, ITenantSer private static readonly string[] ObfuscationFunctions = [ - "CHAR(", "CHR(", "ASCII(", "ORD(", "HEX(", "UNHEX(", "CONV(", - "CONVERT(", "CAST(", "BINARY(", "CONCAT_WS(", "MAKE_SET(", - "ELT(", "FIELD(", "FIND_IN_SET(", "EXPORT_SET(", "LOAD_FILE(", - "FROM_BASE64(", "TO_BASE64(", "COMPRESS(", "UNCOMPRESS(", - "AES_ENCRYPT(", "AES_DECRYPT(", "DES_ENCRYPT(", "DES_DECRYPT(", - "ENCODE(", "DECODE(", "PASSWORD(", "OLD_PASSWORD(" + "CHAR", "CHR", "ASCII", "ORD", "HEX", "UNHEX", "CONV", + "CONVERT", "CAST", "BINARY", "CONCAT_WS", "MAKE_SET", + "ELT", "FIELD", "FIND_IN_SET", "EXPORT_SET", "LOAD_FILE", + "FROM_BASE64", "TO_BASE64", "COMPRESS", "UNCOMPRESS", + "AES_ENCRYPT", "AES_DECRYPT", "DES_ENCRYPT", "DES_DECRYPT", + "ENCODE", "DECODE", "PASSWORD", "OLD_PASSWORD" ]; + // Pre-compiled regex patterns for word-boundary keyword matching + private static readonly Regex DangerousKeywordsPattern = new( + @"\b(" + string.Join("|", DangerousKeywords.Select(Regex.Escape)) + @")\b", + RegexOptions.IgnoreCase | RegexOptions.Compiled, + TimeSpan.FromSeconds(3)); + + private static readonly Regex ObfuscationFunctionsPattern = new( + @"\b(" + string.Join("|", ObfuscationFunctions.Select(Regex.Escape)) + @")\s*\(", + RegexOptions.IgnoreCase | RegexOptions.Compiled, + TimeSpan.FromSeconds(3)); + private async Task GetEntraIdAccessTokenAsync() { lock (_tokenLock) @@ -134,23 +147,21 @@ internal static void ValidateQuerySafety(string query) throw new InvalidOperationException($"Query length exceeds the maximum allowed limit of {MaxResultLimit:N0} characters to prevent potential DoS attacks."); } - // Clean the query: remove comments, normalize whitespace, and trim - var cleanedQuery = query; - - // Remove line comments (-- comment) - cleanedQuery = Regex.Replace(cleanedQuery, @"--.*?$", "", RegexOptions.Multiline); + // Strip string literals before checking for comment markers to avoid + // false positives (e.g., 'C#Developer' or 'foo--bar' are not comments). + // The pattern handles both SQL-standard doubled quotes ('') and + // MySQL's default backslash escaping (\') inside string literals. + var queryWithoutStrings = Regex.Replace(query, "'([^'\\\\]|\\\\.|'')*'", "'str'", RegexOptions.None, TimeSpan.FromSeconds(3)); - // Remove hash comments (# comment) - cleanedQuery = Regex.Replace(cleanedQuery, @"#.*?$", "", RegexOptions.Multiline); - - // Remove block comments (/* comment */) - cleanedQuery = Regex.Replace(cleanedQuery, @"/\*.*?\*/", "", RegexOptions.Singleline); - - // Normalize whitespace: replace multiple whitespace characters with single space - cleanedQuery = Regex.Replace(cleanedQuery, @"\s+", " ", RegexOptions.Multiline); + // Reject queries containing SQL comments to prevent bypass attacks + // (e.g., MySQL version-specific comments /*!50000 ... */ that are executed as code) + if (queryWithoutStrings.Contains("--", StringComparison.Ordinal) || queryWithoutStrings.Contains("/*", StringComparison.Ordinal) || queryWithoutStrings.Contains("#", StringComparison.Ordinal)) + { + throw new InvalidOperationException("SQL comments are not allowed for security reasons."); + } - // Trim the result - cleanedQuery = cleanedQuery.Trim(); + // Normalize whitespace and trim for validation + var cleanedQuery = Regex.Replace(query, @"\s+", " ", RegexOptions.Multiline).Trim(); // Ensure the cleaned query is not empty if (string.IsNullOrWhiteSpace(cleanedQuery)) @@ -169,34 +180,28 @@ internal static void ValidateQuerySafety(string query) throw new InvalidOperationException("Multiple SQL statements are not allowed. Use only a single SELECT statement."); } - // List of dangerous SQL keywords that should be blocked - var queryUpper = cleanedQuery.ToUpperInvariant(); - - foreach (var keyword in DangerousKeywords) + // List of dangerous SQL keywords that should be blocked (word-boundary matching) + var keywordMatch = DangerousKeywordsPattern.Match(cleanedQuery); + if (keywordMatch.Success) { - if (queryUpper.Contains(keyword)) - { - throw new InvalidOperationException($"Query contains dangerous keyword '{keyword}' which is not allowed for security reasons."); - } + throw new InvalidOperationException($"Query contains dangerous keyword '{keywordMatch.Value.ToUpperInvariant()}' which is not allowed for security reasons."); } // Check for character conversion functions that may be used for obfuscation - foreach (var func in ObfuscationFunctions) + var funcMatch = ObfuscationFunctionsPattern.Match(cleanedQuery); + if (funcMatch.Success) { - if (queryUpper.Contains(func)) - { - throw new InvalidOperationException($"Character conversion and obfuscation functions like '{func.TrimEnd('(')}' are not allowed for security reasons."); - } + throw new InvalidOperationException($"Character conversion and obfuscation functions like '{funcMatch.Groups[1].Value.ToUpperInvariant()}' are not allowed for security reasons."); } // Additional validation: Only allow SELECT statements - var trimmedQuery = queryUpper.Trim(); + var trimmedQuery = cleanedQuery.Trim(); var allowedStartPatterns = new[] { "SELECT" }; - bool isAllowed = allowedStartPatterns.Any(pattern => trimmedQuery.StartsWith(pattern)); + bool isAllowed = allowedStartPatterns.Any(pattern => trimmedQuery.StartsWith(pattern, StringComparison.OrdinalIgnoreCase)); if (!isAllowed) { diff --git a/tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.UnitTests/Services/MySqlServiceQueryValidationTests.cs b/tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.UnitTests/Services/MySqlServiceQueryValidationTests.cs index 67c61d9c94..5052626fb9 100644 --- a/tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.UnitTests/Services/MySqlServiceQueryValidationTests.cs +++ b/tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.UnitTests/Services/MySqlServiceQueryValidationTests.cs @@ -30,7 +30,19 @@ public MySqlServiceQueryValidationTests() [InlineData("SELECT * FROM users LIMIT 100")] [InlineData("SELECT COUNT(*) FROM products LIMIT 1")] [InlineData("SELECT COUNT(*) FROM products;")] - [InlineData("SELECT COUNT(*) FROM products; -- comment")] + [InlineData("SELECT * FROM users WHERE name = 'C#Developer'")] + [InlineData("SELECT * FROM tags WHERE value LIKE '%#sale%'")] + [InlineData("SELECT * FROM users WHERE name = 'foo--bar'")] + [InlineData("SELECT * FROM users WHERE name = 'it\\'s a test -- ok'")] + [InlineData("SELECT * FROM users WHERE name = 'back\\\\slash'")] + [InlineData("SELECT * FROM user_deletions")] + [InlineData("SELECT * FROM datasets")] + [InlineData("SELECT * FROM skills")] + [InlineData("SELECT * FROM grants")] + [InlineData("SELECT * FROM reunion_events")] + [InlineData("SELECT preset FROM config")] + [InlineData("SELECT * FROM committees")] + [InlineData("SELECT VARCHAR(col) FROM t")] public void ValidateQuerySafety_WithSafeQueries_ShouldNotThrow(string query) { // Act & Assert - Should not throw any exception @@ -102,9 +114,7 @@ public void ValidateQuerySafety_WithLongQuery_ShouldThrowInvalidOperationExcepti } [Theory] - [InlineData("SELECT * FROM users; DROP TABLE users")] [InlineData("SELECT * FROM users; SELECT * FROM products")] - [InlineData("SELECT * FROM users; SELECT * FROM products; --comment")] [InlineData("SELECT * FROM Logs; union select password from Users")] public void ValidateQuerySafety_WithMultipleStatements_ShouldThrowInvalidOperationException(string query) { @@ -128,4 +138,29 @@ public void ValidateQuerySafety_WithObfuscationFunctions_ShouldThrowInvalidOpera exception.Message.Contains("dangerous keyword"), $"Expected obfuscation or keyword validation error, but got: {exception.Message}"); } + + [Theory] + [InlineData("SELECT 1 -- line comment")] + [InlineData("SELECT 1 /* block comment */")] + [InlineData("SELECT 1 /*!50000 UNION SELECT user FROM mysql.user */")] + [InlineData("SELECT 1 # hash comment")] + public void ValidateQuerySafety_WithComments_ShouldThrowInvalidOperationException(string query) + { + // Act & Assert + var exception = Assert.Throws(() => MySqlService.ValidateQuerySafety(query)); + + Assert.Contains("SQL comments are not allowed for security reasons", exception.Message); + } + + [Theory] + [InlineData("SELECT 1 UNION SELECT user FROM mysql.user")] + [InlineData("SELECT 1 INTERSECT SELECT 2")] + [InlineData("SELECT 1 EXCEPT SELECT 2")] + public void ValidateQuerySafety_WithSetOperations_ShouldThrowInvalidOperationException(string query) + { + // Act & Assert + var exception = Assert.Throws(() => MySqlService.ValidateQuerySafety(query)); + + Assert.Contains("dangerous keyword", exception.Message); + } } From 0b5f572c3367dd68519734f4b365a55266b0cc16 Mon Sep 17 00:00:00 2001 From: Xiang Yan Date: Wed, 18 Mar 2026 16:08:10 -0700 Subject: [PATCH 2/2] Update servers/Azure.Mcp.Server/CHANGELOG.md Co-authored-by: vcolin7 --- servers/Azure.Mcp.Server/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/servers/Azure.Mcp.Server/CHANGELOG.md b/servers/Azure.Mcp.Server/CHANGELOG.md index 36ab5d1b3f..22463f6396 100644 --- a/servers/Azure.Mcp.Server/CHANGELOG.md +++ b/servers/Azure.Mcp.Server/CHANGELOG.md @@ -10,7 +10,7 @@ The Azure MCP Server updates automatically by default whenever a new release com ### Bugs Fixed -- Hardened Postgres SQL query validator to block set-operation keywords (UNION, INTERSECT, EXCEPT), additional dangerous system catalogs, and fixed false-positive comment detection inside string literals +- Hardened Postgres SQL query validator to block set-operation keywords (UNION, INTERSECT, EXCEPT), additional dangerous system catalogs, and fixed false-positive comment detection inside string literals [[#2097](https://github.com/microsoft/mcp/pull/2097)] - Fixed SQL injection vulnerability in MySQL query validation that allowed bypassing safety checks via version-specific comments and UNION-based attacks. [[#2083](https://github.com/microsoft/mcp/pull/2083)] ### Other Changes