From 3cb7cb5622543c42141818bd32ab834f1b86505e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 9 Jan 2026 00:10:16 +0000 Subject: [PATCH 1/3] Initial plan From 7a957b64cfcd81e57fb4b7816d65e79727d78c5e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 9 Jan 2026 00:14:18 +0000 Subject: [PATCH 2/3] Address PR review comments on SSRF DNS validation - Enhanced exception handling to catch OSError in addition to socket.gaierror - Added comprehensive IP range checks (link_local, reserved, multicast) - Strip IPv6 zone identifiers before validation - Document TOCTOU vulnerability limitation in code comments and sentinel.md Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com> --- .jules/sentinel.md | 9 ++++++--- main.py | 16 ++++++++++++---- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index f6ee4ff..21bcbfd 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -42,8 +42,11 @@ ## 2025-01-24 - [SSRF via DNS Resolution] **Vulnerability:** The `validate_folder_url` function blocked private IP literals but failed to resolve domain names to check their underlying IPs. An attacker could use a domain (e.g., `local.test`) that resolves to a private IP (e.g., `127.0.0.1`) to bypass the SSRF protection and access internal services. -**Learning:** Blocking IP literals is insufficient for SSRF protection. DNS resolution must be performed to verify that a domain does not resolve to a restricted IP address. +**Learning:** Blocking IP literals is insufficient for SSRF protection. DNS resolution must be performed to verify that a domain does not resolve to a restricted IP address. Additionally, comprehensive checks are needed for all dangerous IP ranges (private, loopback, link-local, reserved, multicast). **Prevention:** 1. Resolve domain names using `socket.getaddrinfo` before making requests. -2. Verify all resolved IP addresses against private/loopback ranges. -3. Handle DNS resolution failures securely (fail-closed). +2. Verify all resolved IP addresses against private/loopback/link-local/reserved/multicast ranges. +3. Handle DNS resolution failures securely (fail-closed), catching both `socket.gaierror` and `OSError`. +4. Strip IPv6 zone identifiers before IP validation. +**Known Limitations:** +- DNS rebinding attacks (TOCTOU): An attacker's DNS server could return a safe IP during validation and a private IP during the actual request. This is a fundamental limitation of DNS-based SSRF protection. diff --git a/main.py b/main.py index 9ccdfdb..a59a228 100644 --- a/main.py +++ b/main.py @@ -211,16 +211,24 @@ def validate_folder_url(url: str) -> bool: return False except ValueError: # Not an IP literal, it's a domain. Resolve to check for private IPs. + # Note: This check has a Time-Of-Check-Time-Of-Use (TOCTOU) limitation. + # DNS rebinding attacks could return a safe IP during validation and a + # private IP during the actual request. This is a known limitation of + # DNS-based SSRF protection. try: # Resolve hostname to IPs addr_infos = socket.getaddrinfo(hostname, None) for family, kind, proto, canonname, sockaddr in addr_infos: ip_str = sockaddr[0] - resolved_ip = ipaddress.ip_address(ip_str) - if resolved_ip.is_private or resolved_ip.is_loopback: - log.warning(f"Skipping unsafe URL (domain {sanitize_for_log(hostname)} resolves to private IP {resolved_ip}): {sanitize_for_log(url)}") + # Strip IPv6 zone identifier (e.g., "%eth0") if present + ip_no_zone = ip_str.split('%', 1)[0] + resolved_ip = ipaddress.ip_address(ip_no_zone) + if (resolved_ip.is_private or resolved_ip.is_loopback or + resolved_ip.is_link_local or resolved_ip.is_reserved or + resolved_ip.is_multicast): + log.warning(f"Skipping unsafe URL (domain {sanitize_for_log(hostname)} resolves to restricted IP {resolved_ip}): {sanitize_for_log(url)}") return False - except socket.gaierror: + except (socket.gaierror, OSError): log.warning(f"Could not resolve hostname {sanitize_for_log(hostname)}, treating as unsafe.") return False From 3a3b0631ce11b65fd2aba02bf95f46f7a6503862 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 9 Jan 2026 00:15:18 +0000 Subject: [PATCH 3/3] Add comprehensive IP checks to literal IP validation for consistency Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com> --- main.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/main.py b/main.py index a59a228..b69c2c9 100644 --- a/main.py +++ b/main.py @@ -206,8 +206,10 @@ def validate_folder_url(url: str) -> bool: try: ip = ipaddress.ip_address(hostname) - if ip.is_private or ip.is_loopback: - log.warning(f"Skipping unsafe URL (private IP): {sanitize_for_log(url)}") + if (ip.is_private or ip.is_loopback or + ip.is_link_local or ip.is_reserved or + ip.is_multicast): + log.warning(f"Skipping unsafe URL (restricted IP): {sanitize_for_log(url)}") return False except ValueError: # Not an IP literal, it's a domain. Resolve to check for private IPs.