Open
Conversation
xiangyan99
requested review from
JasonYeMSFT,
chidozieononiwu,
fanyang-mono,
g2vinay,
hallipr,
jairmyree and
weshaggard
Contributor
There was a problem hiding this comment.
Pull request overview
Strengthens EndpointValidator SSRF protections by expanding detection of IPv6-based and wildcard-DNS based bypass techniques, and adds/updates unit tests plus a changelog entry to document the security hardening.
Changes:
- Block common wildcard DNS services (
nip.io,sslip.io,xip.io) as reserved hostnames inValidatePublicTargetUrl. - Harden
IsPrivateOrReservedIPto normalize IPv4-mapped IPv6 and detect additional IPv6 non-routable/reserved cases (multicast, discard prefix, documentation prefix, 6to4, Teredo, IPv4-compatible IPv6). - Add unit tests covering IPv4-mapped IPv6, IPv6 reserved ranges, 6to4/Teredo embedding cases, and wildcard DNS blocking; add changelog entry.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| servers/Azure.Mcp.Server/changelog-entries/1773685856512.yaml | Documents SSRF hardening for IPv4-mapped IPv6 bypass handling. |
| core/Microsoft.Mcp.Core/src/Helpers/EndpointValidator.cs | Expands reserved hostname blocking and strengthens IP range classification for SSRF prevention. |
| core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Core.UnitTests/Helpers/EndpointValidatorTests.cs | Adds/updates tests for new SSRF bypass protections and broader reserved IP detection. |
core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Core.UnitTests/Helpers/EndpointValidatorTests.cs
Show resolved
Hide resolved
Add NAT64 (64:ff9b::/96) and NAT64v2 (64:ff9b:1::/48) checks with
RFC 6052 Section 2.2 correct byte extraction for embedded IPv4.
Fix trailing-dot FQDN bypass where 'nip.io.' evaded the reserved
hostname blocklist by normalizing with TrimEnd('.').
Add missing IPv6 ranges: BMWG benchmarking (2001:2::/48),
site-local (fec0::/10).
Add missing IPv4 ranges: TEST-NET-1/2/3, benchmarking (198.18/15),
IANA special (192.0.0/24), 6to4 relay (192.88.99/24).
157 EndpointValidator tests, all passing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…add 48 tests Findings from MQ triple-model analysis (Opus + Codex + test coverage): - CWE-209: Sanitize SecurityException messages to not leak resolved private IP addresses or DNS resolver internals to callers - CWE-697: Remove duplicate BMWG check that over-blocked /32 instead of correct /48 (RFC 5180); the second correct /48 check was dead code - CWE-770: Move reservedHosts array to static readonly field to avoid per-call allocation on validation hot path - Add 48 new test cases: public IPv4/IPv6 baselines, IPv4-mapped and IPv4-compatible public address tests, link-local IPv6, IPv4 range boundary tests, multicast/reserved IPv4, trailing-dot for all reserved hosts, IPv6 transition mechanism URLs via ValidatePublicTargetUrl, subdomain-of-reserved-host blocking, error message sanitization verification - Fix misleading Teredo test comment (described wrong bytes/IP) 205 tests passing (up from 157). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Finding: MQ-HACK-002 (Codex adversarial pass) Severity: Medium CWE: CWE-184 (Incomplete List of Disallowed Inputs), CWE-703 Persona: Bug Bounty Hunter Fix class: AUTO-FIX ISATAP (RFC 5214) embeds IPv4 in the last 4 bytes of an IPv6 address with the interface ID marker 0:5efe (or 02:00:5efe with u/l bit set). Link-local ISATAP (fe80::5efe:...) was already blocked by the fe80::/10 check, but global-prefix ISATAP addresses (e.g., 2001:db8::5efe:a9fe:a9fe) could bypass all checks and tunnel to IMDS. Also adds fail-closed guard for empty DNS AddressList — if GetHostEntry returns zero addresses, the hostname is now rejected instead of silently passing validation. 211 tests passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request strengthens EndpointValidator SSRF defenses (notably around IPv6 transition mechanisms, wildcard DNS services, and error-message sanitization) and expands unit test coverage to validate the new protection behavior.
Changes:
- Harden
ValidatePublicTargetUrlby enforcing HTTP/HTTPS schemes, blocking wildcard DNS/reserved hostnames (with trailing-dot normalization), failing closed on empty DNS results, and sanitizing exception messages. - Expand
IsPrivateOrReservedIPto detect additional private/reserved IPv4/IPv6 ranges and IPv6 transition-mechanism embeddings (IPv4-mapped, 6to4, Teredo, NAT64, SIIT/translated, ISATAP, etc.). - Add extensive unit tests covering the new reserved-range logic and SSRF bypass vectors; add changelog entries for the fix.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| servers/Azure.Mcp.Server/changelog-entries/ssrf-ipv6-hardening.yaml | Adds release note describing the expanded SSRF hardening. |
| servers/Azure.Mcp.Server/changelog-entries/1773685856512.yaml | Adds an additional (potentially duplicate) release note about IPv4-mapped IPv6 SSRF bypass. |
| core/Microsoft.Mcp.Core/src/Helpers/EndpointValidator.cs | Implements SSRF hardening: scheme enforcement, reserved host blocklist + trailing-dot normalization, sanitized DNS errors, expanded reserved/private IP detection for IPv6 transition mechanisms. |
| core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Core.UnitTests/Helpers/EndpointValidatorTests.cs | Adds many tests covering IPv6 transition mechanisms, additional reserved ranges, wildcard DNS blocking, trailing-dot normalization, and sanitized errors. |
core/Microsoft.Mcp.Core/tests/Microsoft.Mcp.Core.UnitTests/Helpers/EndpointValidatorTests.cs
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
[Provide a clear, concise description of the changes]Update endpoint validator to handle IPv6
[Any additional context, screenshots, or information that helps reviewers]GitHub issue number?
[Link to the GitHub issue this PR addresses]Pre-merge Checklist
servers/Azure.Mcp.Server/CHANGELOG.mdand/orservers/Fabric.Mcp.Server/CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationeng/scripts/Process-PackageReadMe.ps1. See Package README/servers/Azure.Mcp.Server/docs/azmcp-commands.mdand/or/docs/fabric-commands.md.\eng\scripts\Update-AzCommandsMetadata.ps1to update tool metadata in azmcp-commands.md (required for CI)ToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.jsonbreaking-changelabel/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline