MySQL security enhancement

servers/Azure.Mcp.Server/CHANGELOG.md

@@ -10,7 +10,8 @@ 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
 

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<string> 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)
         {

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<InvalidOperationException>(() => 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<InvalidOperationException>(() => MySqlService.ValidateQuerySafety(query));
+
+        Assert.Contains("dangerous keyword", exception.Message);
+    }
 }