Fixed input validation issue in MySql and Postgres

[Larry Osterman (LarryOsterman)]

What does this PR do?

Fixes two input validation issues in the MySql and Postgres SQL tools.

GitHub issue number?

#3203

Pre-merge Checklist

• Required for All PRs
  • Read contribution guidelines
  • PR title clearly describes the change
  • Commit history is clean with descriptive messages (cleanup guide)
  • Added comprehensive tests for new/modified functionality
  • Created a changelog entry if the change falls among the following: new feature, bug fix, UI/UX update, breaking change, or updated dependencies. Follow the changelog entry guide
• For MCP tool changes:
  • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
  • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
  • Validate README.md changes running the script ./eng/scripts/Process-PackageReadMe.ps1. See Package README
  • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
  • For tools with new names, including new tools or renamed tools, update consolidated-tools.json
  • For renamed tools, follow the Tool Rename Checklist and tag the PR with the breaking-change label
  • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
• Extra steps for Azure MCP Server tool changes:
  • Updated command list in servers/Azure.Mcp.Server/docs/azmcp-commands.md
  • Ran ./eng/scripts/Update-AzCommandsMetadata.ps1 to update tool metadata in azmcp-commands.md (required for CI)
  • Updated test prompts in servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
  • 👉 For Community (non-Microsoft team member) PRs:
    • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
    • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR tightens SQL query safety validation for the Postgres and MySQL toolsets, aiming to prevent obfuscation-based bypasses of the existing “read-only SELECT” enforcement.

Changes:

• Postgres: decode U& Unicode escape sequences during validation and add test coverage for obfuscated dangerous identifiers.
• MySQL: broaden literal-stripping logic (including N'...' and hex forms) and add tests covering national strings, hex literals, and dangerous function calls.
• Add new unit tests covering the introduced validation scenarios.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tools/Azure.Mcp.Tools.Postgres/tests/Azure.Mcp.Tools.Postgres.Tests/Validation/SqlQueryValidatorTests.cs	Adds tests for Unicode-escaped dangerous identifiers and benign Unicode-escaped identifiers.
tools/Azure.Mcp.Tools.Postgres/src/Validation/SqlQueryValidator.cs	Introduces Unicode-escape decoding and expands literal stripping logic for Postgres validation.
tools/Azure.Mcp.Tools.MySql/tests/Azure.Mcp.Tools.MySql.Tests/Services/MySqlServiceQueryValidationTests.cs	Adds tests for N'...' strings, hex literals, and dangerous function calls.
tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs	Expands literal stripping and introduces national-string escape decoding for MySQL validation.

Suppressed comments (2)

tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs:199

• ValidateQuerySafety computes decodedQuery but then continues validation (whitespace normalization, multiple-statement detection, dangerous keyword/function checks, and SELECT-only enforcement) against the original query. This means the new decoding step does not actually affect the main safety checks it claims to support.

This issue also appears on line 262 of the same file.

        // Decode escape sequences in National/Unicode strings (N'...') to detect obfuscated function names.
        // Example attack vectors this prevents:
        // - N'pg_sl\0065ep' with escape sequences → pg_sleep
        // - SELECT N'CHAR(0x...) encoded function names' → binary-encoded dangerous functions
        var decodedQuery = DecodeNationalStringEscapes(query);

        // Strip string literals and hex literals before checking for comment markers to avoid
        // false positives (e.g., 'C#Developer' or 'foo--bar' are not comments).
        // The pattern handles:
        // - Standard quoted strings: 'text' with doubled quotes ('') and backslash escaping (\')
        // - National/Unicode strings: N'text' (MySQL's Unicode support)
        // - Hex literals: 0xHHHH or X'HHHH' (binary function name encoding)
        var queryWithoutStrings = Regex.Replace(decodedQuery, "[nN]'([^'\\\\]|\\\\.|'')*'|'([^'\\\\]|\\\\.|'')*'|0x[0-9A-Fa-f]+|[xX]'[0-9A-Fa-f]*'", "'str'", RegexOptions.None, RegexHelper.DefaultRegexTimeout);

        // 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.");
        }

        // Normalize whitespace and trim for validation
        var cleanedQuery = Regex.Replace(query, @"\s+", " ", RegexOptions.Multiline).Trim();

tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs:275

• DecodeNationalStringEscapes decodes ' to a raw single-quote and then re-wraps the content as N'...'. That produces an invalid SQL string literal (the quote terminates the literal early) and can break the subsequent regex-based stripping/comment detection logic. The decoded content should preserve a valid string literal representation (e.g., SQL-standard doubled quotes).

                // Decode backslash escape sequences
                var decoded = content
                    .Replace("\\\\", "\x00")  // Temp marker for backslash
                    .Replace("\\'", "'")
                    .Replace("\\\"", "\"")
                    .Replace("\\n", "\n")
                    .Replace("\\r", "\r")
                    .Replace("\\t", "\t")
                    .Replace("\\b", "\b")
                    .Replace("\\f", "\f")
                    .Replace("\\0", "\0")
                    .Replace("\x00", "\\");   // Restore backslash
                return $"N'{decoded}'";
            },

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

tools/Azure.Mcp.Tools.MySql/src/Services/MySqlService.cs:276

• DecodeNationalStringEscapes currently decodes ' to a raw single-quote and then re-emits it as N'…' without re-escaping. That can produce syntactically invalid SQL (e.g., N'it's' → N'it's'), which can break the subsequent string-stripping regex and lead to incorrect validation results. Also, this Regex.Replace overload omits RegexHelper.DefaultRegexTimeout (potential ReDoS risk).

                return $"N'{decoded}'";
            },
            RegexOptions.Compiled);

[Vinay Gera (g2vinay)]

+1

[Vinay Gera (g2vinay)]

PostgreSQL lets the escape character be redefined with a trailing UESCAPE 'x' clause.
We will also need to handle this, I think.

-- DoS via pg_sleep, executable by any principal
SELECT U&"pg_slee!0070" UESCAPE '!' (10);

[Larry Osterman (LarryOsterman)]

Wow, good catch. I think that the copilot suggested fix immediately above this resolves the issue by disabling the UESCAPE pattern entirely.

Unless you think that the UESCAPE pattern is something that is common...

[Vinay Gera (g2vinay)]

yeah, disallowing it is fine, addressing copilot comment will cover it.