Skip to content

fix(validators): don't flag quoted URL schemes (://) as polyglot attacks - #6418

Open
YMahut wants to merge 1 commit into
IBM:mainfrom
YMahut:fix/polyglot-url-scheme-false-positive
Open

fix(validators): don't flag quoted URL schemes (://) as polyglot attacks#6418
YMahut wants to merge 1 commit into
IBM:mainfrom
YMahut:fix/polyglot-url-scheme-false-positive

Conversation

@YMahut

@YMahut YMahut commented Aug 26, 2026

Copy link
Copy Markdown

📌 Summary

SecurityValidator.sanitize_display_text rejects any display text in which a quoted string contains // (via the _POLYGLOT_PATTERNS entry ["'].*//["']). Legitimate URLs get caught: a tool whose description enumerates connection-string examples such as "postgresql://" / "mysql://" fails validation with <field> contains potentially dangerous character sequences, which blocks tool registration.

🔁 Reproduction Steps

Closes #6419

from mcpgateway.common.validators import SecurityValidator
SecurityValidator.sanitize_display_text('Examples: "postgresql://" or "mysql://"', "desc")
# ValueError: desc contains potentially dangerous character sequences

Observed in practice with a Superset "create connection" tool whose description lists the supported DB URL schemes.

🐞 Root Cause

The // of a URL scheme separator (://) is treated as a JS line-comment polyglot by ["'].*//["'] in mcpgateway/common/validators.py.

💡 Fix Description

Add a negative lookbehind so a scheme :// is not matched, while a bare // before a closing quote (a genuine JS line-comment polyglot) is still caught:

["'].*//["']   ->   ["'].*(?<!:)//["']

The HTML/JS patterns and the other polyglot patterns are unchanged, so real XSS/polyglot payloads remain blocked.

📏 Reviewability

  • This PR has one clear purpose
  • The linked issue is not labeled triage
  • Unrelated bugs or improvements are tracked in separate issues/PRs
  • Tests are included with the code they validate
  • If AI-assisted, I understand and can explain the generated changes

🧪 Verification

Two regression tests added in tests/unit/mcpgateway/validation/test_validators.py:

  • test_sanitize_display_text_url_scheme_allowed — quoted URL schemes are accepted;
  • test_sanitize_display_text_polyglot_double_slash_still_blocked — a bare // before a quote is still rejected.
Check Command Status
Lint suite make lint not run in submitter environment
Unit tests make test not run in submitter environment
Coverage ≥ 80 % make coverage not run in submitter environment
Manual regression standalone re check verified "postgresql://"/"mysql://" no longer flagged and "a//" still flagged; existing HTML/JS/polyglot checks unaffected — relying on project CI for the full suite

📐 MCP Compliance (if relevant)

N/A — input-validation refinement, no change to the MCP protocol or client behavior.

✅ Checklist

  • Code formatted (make black isort pre-commit) — not run locally; change is a 1-line regex + tests (formatting-neutral)
  • No secrets/credentials committed

The polyglot detection pattern ['"].*//['"] in SecurityValidator matched any
quoted string containing "//", including legitimate URLs. Tool descriptions
with connection-string examples (e.g. "postgresql://", "mysql://") were
rejected during registration with "contains potentially dangerous character
sequences".

Add a negative lookbehind (?<!:) so a URL scheme separator (://) is not treated
as a polyglot. A bare "//" before a closing quote (JS line-comment polyglot) is
still detected. Adds regression tests.

Signed-off-by: YouriM <youri.mahut@bjtmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sanitize_display_text: quoted URL schemes ("://") flagged as polyglot attacks

2 participants