🛡️ Sentinel: [HIGH] Fix DNS Rebinding / SSRF Vulnerability#106
🛡️ Sentinel: [HIGH] Fix DNS Rebinding / SSRF Vulnerability#106google-labs-jules[bot] wants to merge 1 commit intomainfrom
Conversation
- Added `socket` import to `main.py`. - Updated `validate_folder_url` to resolve hostnames and check against private/loopback IPs. - Added `tests/test_security.py` to verify the fix. - Updated `.jules/sentinel.md` with security learning.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Merging to
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 1. Parse URLs and check hostnames against `localhost` and private IP ranges using `ipaddress` module. | ||
| 2. Enforce strict length limits on user inputs (e.g., profile IDs) to prevent resource exhaustion or buffer abuse. | ||
|
|
||
| ## 2025-02-23 - [DNS Rebinding / SSRF via Domain Resolution] |
Check notice
Code scanning / Remark-lint (reported by Codacy)
Warn when references to undefined definitions are found. Note
| 1. Parse URLs and check hostnames against `localhost` and private IP ranges using `ipaddress` module. | ||
| 2. Enforce strict length limits on user inputs (e.g., profile IDs) to prevent resource exhaustion or buffer abuse. | ||
|
|
||
| ## 2025-02-23 - [DNS Rebinding / SSRF via Domain Resolution] |
Check notice
Code scanning / Remark-lint (reported by Codacy)
Warn when shortcut reference links are used. Note
| try: | ||
| # Resolve hostname to check if it points to a private IP | ||
| addr_infos = socket.getaddrinfo(hostname, None) | ||
| for family, kind, proto, canonname, sockaddr in addr_infos: |
Check warning
Code scanning / Prospector (reported by Codacy)
Unused variable 'family' (unused-variable) Warning
| 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 resolves to private IP {ip_str}): {sanitize_for_log(url)}") |
Check warning
Code scanning / Prospector (reported by Codacy)
Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning
| if resolved_ip.is_private or resolved_ip.is_loopback: | ||
| log.warning(f"Skipping unsafe URL (domain resolves to private IP {ip_str}): {sanitize_for_log(url)}") | ||
| return False | ||
| except (socket.gaierror, ValueError) as e: |
Check warning
Code scanning / Prospector (reported by Codacy)
local variable 'e' is assigned to but never used (F841) Warning
| log.warning(f"Skipping unsafe URL (domain resolves to private IP {ip_str}): {sanitize_for_log(url)}") | ||
| return False | ||
| except (socket.gaierror, ValueError) as e: | ||
| log.warning(f"Skipping unsafe URL (DNS resolution failed): {sanitize_for_log(url)}") |
Check warning
Code scanning / Prospector (reported by Codacy)
Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning
| log.warning(f"Skipping unsafe URL (DNS resolution failed): {sanitize_for_log(url)}") | ||
| return False | ||
| except Exception as e: | ||
| log.warning(f"Skipping unsafe URL (DNS check error): {e}") |
Check warning
Code scanning / Prospector (reported by Codacy)
Use lazy % formatting in logging functions (logging-fstring-interpolation) Warning
| # Configure logging to avoid polluting output | ||
| logging.basicConfig(level=logging.CRITICAL) | ||
|
|
||
| import main |
Check warning
Code scanning / Prospector (reported by Codacy)
Reimport 'main' (imported line 9) (reimported) Warning test
| # Should be False (Secure) | ||
| self.assertFalse(result, "validate_folder_url should return False for domains resolving to private IPs") | ||
|
|
||
| if __name__ == "__main__": |
Check warning
Code scanning / Prospector (reported by Codacy)
expected 2 blank lines after class or function definition, found 1 (E305) Warning test
| 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 resolves to private IP {ip_str}): {sanitize_for_log(url)}") |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Line too long (125/100) Warning
| if resolved_ip.is_private or resolved_ip.is_loopback: | ||
| log.warning(f"Skipping unsafe URL (domain resolves to private IP {ip_str}): {sanitize_for_log(url)}") | ||
| return False | ||
| except (socket.gaierror, ValueError) as e: |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Variable name "e" doesn't conform to snake_case naming style Warning
| except (socket.gaierror, ValueError) as e: | ||
| log.warning(f"Skipping unsafe URL (DNS resolution failed): {sanitize_for_log(url)}") | ||
| return False | ||
| except Exception as e: |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Variable name "e" doesn't conform to snake_case naming style Warning
| @@ -0,0 +1,35 @@ | |||
| import unittest | |||
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Missing module docstring Warning test
| # Configure logging to avoid polluting output | ||
| logging.basicConfig(level=logging.CRITICAL) | ||
|
|
||
| import main |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Import "import main" should be placed at the top of the module Warning test
| try: | ||
| # Resolve hostname to check if it points to a private IP | ||
| addr_infos = socket.getaddrinfo(hostname, None) | ||
| for family, kind, proto, canonname, sockaddr in addr_infos: |
Check notice
Code scanning / Pylintpython3 (reported by Codacy)
Unused variable 'proto' Note
| try: | ||
| # Resolve hostname to check if it points to a private IP | ||
| addr_infos = socket.getaddrinfo(hostname, None) | ||
| for family, kind, proto, canonname, sockaddr in addr_infos: |
Check notice
Code scanning / Pylintpython3 (reported by Codacy)
Unused variable 'canonname' Note
| 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 resolves to private IP {ip_str}): {sanitize_for_log(url)}") |
Check notice
Code scanning / Pylintpython3 (reported by Codacy)
Use lazy % formatting in logging functions Note
| log.warning(f"Skipping unsafe URL (domain resolves to private IP {ip_str}): {sanitize_for_log(url)}") | ||
| return False | ||
| except (socket.gaierror, ValueError) as e: | ||
| log.warning(f"Skipping unsafe URL (DNS resolution failed): {sanitize_for_log(url)}") |
Check notice
Code scanning / Pylintpython3 (reported by Codacy)
Use lazy % formatting in logging functions Note
| log.warning(f"Skipping unsafe URL (DNS resolution failed): {sanitize_for_log(url)}") | ||
| return False | ||
| except Exception as e: | ||
| log.warning(f"Skipping unsafe URL (DNS check error): {e}") |
Check notice
Code scanning / Pylintpython3 (reported by Codacy)
Use lazy % formatting in logging functions Note
| 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 resolves to private IP {ip_str}): {sanitize_for_log(url)}") |
Check warning
Code scanning / Pylint (reported by Codacy)
Line too long (125/100) Warning
| if resolved_ip.is_private or resolved_ip.is_loopback: | ||
| log.warning(f"Skipping unsafe URL (domain resolves to private IP {ip_str}): {sanitize_for_log(url)}") | ||
| return False | ||
| except (socket.gaierror, ValueError) as e: |
Check warning
Code scanning / Pylint (reported by Codacy)
Variable name "e" doesn't conform to snake_case naming style Warning
| @@ -0,0 +1,35 @@ | |||
| import unittest | |||
Check warning
Code scanning / Pylint (reported by Codacy)
Missing module docstring Warning test
| # Configure logging to avoid polluting output | ||
| logging.basicConfig(level=logging.CRITICAL) | ||
|
|
||
| import main |
Check warning
Code scanning / Pylint (reported by Codacy)
Import "import main" should be placed at the top of the module Warning test
|
|
||
| import main | ||
|
|
||
| class TestSecurity(unittest.TestCase): |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing class docstring Warning test
| # Not an IP literal, it's a domain. | ||
| pass | ||
| # Perform DNS resolution to check for private IPs (DNS Rebinding protection) | ||
| try: |
Check notice
Code scanning / Pylint (reported by Codacy)
Catching too general exception Exception Note
| try: | ||
| # Resolve hostname to check if it points to a private IP | ||
| addr_infos = socket.getaddrinfo(hostname, None) | ||
| for family, kind, proto, canonname, sockaddr in addr_infos: |
Check notice
Code scanning / Pylint (reported by Codacy)
Unused variable 'proto' Note
| try: | ||
| # Resolve hostname to check if it points to a private IP | ||
| addr_infos = socket.getaddrinfo(hostname, None) | ||
| for family, kind, proto, canonname, sockaddr in addr_infos: |
Check notice
Code scanning / Pylint (reported by Codacy)
Unused variable 'kind' Note
| try: | ||
| # Resolve hostname to check if it points to a private IP | ||
| addr_infos = socket.getaddrinfo(hostname, None) | ||
| for family, kind, proto, canonname, sockaddr in addr_infos: |
Check notice
Code scanning / Pylint (reported by Codacy)
Unused variable 'family' Note
| try: | ||
| # Resolve hostname to check if it points to a private IP | ||
| addr_infos = socket.getaddrinfo(hostname, None) | ||
| for family, kind, proto, canonname, sockaddr in addr_infos: |
Check notice
Code scanning / Pylint (reported by Codacy)
Unused variable 'canonname' Note
|
Superseded: SSRF/DNS rebinding protection already in main via PR #109. |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
1 participant
🚨 Severity: HIGH
💡 Vulnerability:
validate_folder_urlallowed domains that resolve to private IPs (e.g., DNS Rebinding), potentially allowing SSRF against internal services.🎯 Impact: An attacker could supply a malicious URL that points to an internal service, bypassing the private IP check.
🔧 Fix: Added DNS resolution (
socket.getaddrinfo) tovalidate_folder_urlto check if the hostname resolves to a private or loopback IP.✅ Verification: Added
tests/test_security.pywhich mocks DNS resolution to a private IP and asserts that validation fails. Verified that valid URLs still work via--dry-run.PR created automatically by Jules for task 1232336768217136543 started by @abhimehro