Merge pull request #85 from abhimehro/copilot/sub-pr-72

Enhance SSRF DNS validation with comprehensive IP checks and IPv6 support

Author: abhimehro

.jules/sentinel.md

@@ -42,6 +42,14 @@
 
 ## 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. 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/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.
 **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. Note that check-time validation has TOCTOU limitations vs request-time verification.
 **Prevention:**
 1. Resolve domain names using `socket.getaddrinfo` before making requests.

main.py

@@ -206,29 +206,29 @@ 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.
+            # 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
                 # Note: This check has a Time-Of-Check-Time-Of-Use (TOCTOU) limitation.
                 # A malicious DNS server could return a safe IP now and a private IP later.
                 addr_infos = socket.getaddrinfo(hostname, None)
                 for family, kind, proto, canonname, sockaddr in addr_infos:
                     ip_str = sockaddr[0]
-
-                    # Strip IPv6 zone identifier if present (e.g., fe80::1%eth0)
-                    if '%' in ip_str:
-                        ip_str = ip_str.split('%', 1)[0]
-
-                    resolved_ip = ipaddress.ip_address(ip_str)
-
-                    if (resolved_ip.is_private or
-                        resolved_ip.is_loopback or
-                        resolved_ip.is_link_local or
-                        resolved_ip.is_reserved or
+                    # 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