GHSA-489G-7RXV-6C8Q: DNS Rebinding SSRF Fix Review in mcp-atlassian

Vulnerability

GHSA-489G-7RXV-6C8Q describes an SSRF bypass in mcp-atlassian where unauthenticated users can supply Jira/Confluence URLs that pass validation but later resolve or redirect to internal resources. The core issue is a DNS rebinding / TOCTOU pattern: validation and network use are separated, so a hostname can appear safe at check time and become unsafe at use time. The patch series in PR #986 adds URL validation and redirect inspection, but the follow-up in PR #1005 changes semantics so allowlisted domains skip DNS resolution checks entirely.

The vulnerable surface is the header-driven base URL selection path in the server, plus any redirect chain followed by the underlying HTTP client. Without binding the validated destination to the actual socket connection, DNS answers can change after validation, and redirects can pivot a request to metadata or RFC1918 targets.

def validate_url_for_ssrf(url: str) -> str | None:
    ...
    allowlist = _get_domain_allowlist()
    if allowlist is not None:
        if not _hostname_matches_allowlist(hostname, allowlist):
            return f"Hostname {hostname} not in allowed domains"

    dns_error = _check_dns_resolution(hostname)
    if dns_error:
        return dns_error

The later patch summary indicates this behavior was altered for allowlisted domains to effectively return success before DNS checks, documented as: allowlisted domains bypass DNS resolution checks and may therefore permit internal/private-IP hosts if trusted by configuration (PR #1005).

Patch

The patch introduces a centralized validate_url_for_ssrf function in src/mcp_atlassian/utils/urls.py and applies it in two important places from PR #986:

• Inbound validation of X-Atlassian-Jira-Url and X-Atlassian-Confluence-Url before authentication flow proceeds.
• A response hook on the requests session to validate redirect Location targets and block internal pivots.

The validator enforces:

• http/https schemes only.
• Hostname presence.
• Explicit hostname blocks such as localhost and metadata.google.internal.
• IP-literal rejection for non-global addresses, including IPv4-mapped IPv6 handling.
• DNS resolution of hostnames with rejection if any resolved address is non-global.
• Optional domain allowlisting via MCP_ALLOWED_URL_DOMAINS.

def hook(response: Any, **kwargs: Any) -> Any:
    if response.is_redirect:
        redirect_url = response.headers.get("Location", "")
        error = validate_fn(redirect_url)
        if error:
            response.close()
            raise ValueError(f"Redirect blocked (SSRF): {error}")
    return response

Tests added in PR #986 cover private IPs, metadata IPs, bad schemes, IPv6 loopback, and redirect blocking. The follow-up tests in PR #1005 explicitly assert that allowlisted domains are accepted even when DNS fails or would resolve to private IP space, confirming a deliberate policy exception.

Review

Pros

• Moves SSRF checks to the actual user-controlled entry points in server request handling, rather than relying on downstream assumptions.
• Adds redirect validation, which closes a common SSRF bypass where an initially safe URL issues a 30x to an internal target.
• Uses ipaddress and is_global to reject a broad set of non-public address classes instead of maintaining a fragile manual CIDR list.
• Handles IPv4-mapped IPv6 addresses, reducing straightforward parser bypasses.
• Includes regression tests for direct internal targets and redirect-based pivots, grounded in the advisory scenario.

Cons

• The fix does not fully eliminate the TOCTOU root cause. Validation still resolves DNS separately from the eventual connection, so it does not cryptographically or mechanically bind the checked IP to the socket actually used.
• PR #1005 weakens the mitigation by making allowlisted domains bypass DNS resolution checks. That means a trusted hostname can still resolve to internal/private space and be accepted by policy.
• The allowlist is domain-based, not endpoint-based. If a trusted domain is compromised, delegated, wildcarded, or internally split-horizon, the patch intentionally permits requests that the original DNS safety check would have blocked.
• The redirect hook validates the Location URL string, but it still does not guarantee that the subsequent request cannot experience rebinding between validation and connect time.
• The snippets do not show connection-level controls such as disabling automatic redirects entirely, pinning resolved IPs, custom DNS resolution with connect-by-IP plus Host/SNI preservation, or post-connect peer verification against validated addresses.
• The explicit blocked hostname list is narrow and should not be relied on as a primary defense; safety mainly depends on IP classification and configuration.

Verdict

Partial fix.

The patch significantly improves the security posture and closes obvious direct-IP and redirect-based SSRF paths documented in the advisory. However, it does not fully solve the DNS rebinding TOCTOU class because validation remains decoupled from the actual network connection, and the follow-up allowlist behavior in PR #1005 explicitly reintroduces acceptance of private/internal resolution for trusted domains. For engineering teams, this should be treated as a meaningful mitigation rather than a complete root-cause eradication.

A stronger long-term remediation would bind validation to transport: resolve once with a trusted resolver, connect to the validated IP directly, preserve Host/SNI as needed, reject redirects by default or re-resolve and re-pin each hop, and keep allowlists from bypassing network-origin safety checks unless the product explicitly intends to support internal destinations.