[Data] Fix SSRF vulnerability in DatabricksUCDatasource external URL … - #65798
[Data] Fix SSRF vulnerability in DatabricksUCDatasource external URL …#65798hoamgh wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces SSRF validation for external URLs in the Databricks UC datasource by verifying that URLs use HTTPS and do not resolve to private, loopback, link-local, or reserved IP addresses. Comprehensive unit tests are also added to cover these validation scenarios. The review feedback highlights a critical security vulnerability where the validation can be bypassed via HTTP redirects, as requests.get follows redirects by default. It is recommended to disable automatic redirects by setting allow_redirects=False and explicitly raising an error for 3xx status codes.
| raw_response = requests.get( | ||
| external_url, auth=None, headers=None | ||
| ) | ||
| raw_response.raise_for_status() |
There was a problem hiding this comment.
By default, requests.get automatically follows HTTP redirects (3xx status codes). If a compromised or malicious Databricks server returns an external URL that redirects to an internal/private IP address (e.g., http://169.254.169.254), the initial _validate_external_url check will be bypassed because it only validates the initial URL. This leads to a redirect-based SSRF vulnerability.
To prevent this, we should disable automatic redirects by setting allow_redirects=False and explicitly raise an error if a 3xx redirect status code is returned.
Additionally, note that there is a potential Time-of-Check to Time-of-Use (TOCTOU) DNS Rebinding vulnerability because the DNS resolution in _validate_external_url is separate from the DNS resolution performed by requests.get. A malicious DNS server could return a public IP during validation and a private IP during the actual request. While resolving DNS rebinding completely in Python requests requires a custom transport adapter or DNS resolver, disabling redirects is a critical first step that mitigates the most common redirect-based SSRF vectors.
| raw_response = requests.get( | |
| external_url, auth=None, headers=None | |
| ) | |
| raw_response.raise_for_status() | |
| raw_response = requests.get( | |
| external_url, auth=None, headers=None, allow_redirects=False | |
| ) | |
| if 300 <= raw_response.status_code < 400: | |
| raise ValueError( | |
| f"Redirection is not allowed for Databricks external URLs " | |
| f"to prevent SSRF attacks, got status: {raw_response.status_code}" | |
| ) | |
| raw_response.raise_for_status() |
637ec83 to
51decd7
Compare
8590809 to
01d8acb
Compare
93923ba to
1eafca4
Compare
1eafca4 to
d99c0f0
Compare
| auth=None, | ||
| headers=None, | ||
| allow_redirects=False, | ||
| ) |
There was a problem hiding this comment.
DNS rebinding bypasses URL validation
Medium Severity
Hostname resolution in _validate_external_url is not bound to the later requests.get. A compromised Databricks response can return an attacker-controlled host that answers with a public IP during validation and a private or link-local IP when the fetch runs, bypassing the SSRF checks.
Reviewed by Cursor Bugbot for commit d99c0f0. Configure here.
99b8d29 to
1a57218
Compare
3d38014 to
e7843c1
Compare
14357c5 to
d2b179a
Compare
f29a0da to
de1f5a4
Compare
…fetching
_validate_external_url() is added to prevent Server-Side Request Forgery
(SSRF) attacks when fetching data from Databricks external links.
The function validates URLs before fetching by:
- Rejecting non-HTTPS schemes.
- Rejecting URLs containing basic authentication ('@' in netloc) to
prevent parser mismatch vulnerabilities (urllib.parse vs urllib3).
- Resolving hostnames and blocking loopback, link-local, reserved,
and multicast IP addresses.
- Allowing RFC1918 private IPs and CGNAT to naturally fall through
to support VPC PrivateLink endpoints per bug review.
- Enforcing allow_redirects=False and raising a ValueError if
a redirect response (3xx) is returned from Databricks.
Fixes ray-project#65669
Signed-off-by: hoamgh <88762703+hoamgh@users.noreply.github.com>
de1f5a4 to
2ddfbdb
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Reviewed by Cursor Bugbot for commit 9115c1a. Configure here.
| f"IP address {ip}, which is blocked to prevent " | ||
| f"SSRF attacks. If you are using Databricks VPC " | ||
| f"PrivateLink, set RAY_DATABRICKS_ALLOW_PRIVATE_IPS=1." | ||
| ) |
There was a problem hiding this comment.
SSRF filter misses CGNAT address range
Medium Severity
_validate_external_url treats an address as safe unless it is loopback, link-local, reserved, multicast, unspecified, or is_private. In Python, 100.64.0.0/10 is neither is_private nor is_global, so CGNAT and similar internal addresses pass, including well-known cloud metadata such as 100.100.100.200. A compromised Databricks response can still send workers at those hosts.
Reviewed by Cursor Bugbot for commit 9115c1a. Configure here.


Fix SSRF vulnerability in
DatabricksUCDatasource(fixes #65669).databricks_uc_datasource.pyunconditionally callsrequests.get(external_url)with a URL from the Databricks API response JSON, without any SSRF validation.
A compromised or MITM'd server could redirect cluster nodes to internal services
(e.g., cloud metadata at
169.254.169.254) for data exfiltration.Changes:
_validate_external_url()that enforces HTTPS-only and blocksprivate/loopback/link-local/reserved IPs via DNS resolution
and end-to-end SSRF prevention
Related issue number
Fixes #65669
Checks
scripts/format.shto lint the changes in this PR