sha174n commented on code in PR #39301:
URL: https://github.com/apache/superset/pull/39301#discussion_r3294565361
##########
superset/utils/network.py:
##########
@@ -14,14 +14,55 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import ipaddress
import platform
import socket
import subprocess
+# Networks that must never be reached via user-supplied hostnames.
+# Includes loopback, RFC-1918 private ranges, link-local (covers cloud
+# metadata endpoints such as 169.254.169.254), and IPv6 equivalents.
+_SSRF_UNSAFE_NETWORKS = (
+ ipaddress.ip_network("0.0.0.0/8"),
+ ipaddress.ip_network("10.0.0.0/8"),
+ ipaddress.ip_network("127.0.0.0/8"),
+ ipaddress.ip_network("169.254.0.0/16"),
+ ipaddress.ip_network("172.16.0.0/12"),
+ ipaddress.ip_network("192.168.0.0/16"),
+ ipaddress.ip_network("::1/128"),
+ ipaddress.ip_network("fc00::/7"),
+ ipaddress.ip_network("fe80::/10"),
+)
+
PORT_TIMEOUT = 5
PING_TIMEOUT = 5
+def is_safe_host(host: str) -> bool:
+ """
+ Return True if ``host`` resolves exclusively to public, globally-routable
+ IP addresses.
+
+ Returns False if any resolved address falls within a private, loopback,
+ link-local, or otherwise non-routable range. An unresolvable host also
+ returns False.
+ """
+ try:
+ results = socket.getaddrinfo(host, None)
+ except socket.gaierror:
+ return False
+ if not results:
+ return False
+ for _, _, _, _, sockaddr in results:
+ try:
+ ip = ipaddress.ip_address(sockaddr[0])
+ except ValueError:
+ return False
+ if any(ip in net for net in _SSRF_UNSAFE_NETWORKS):
+ return False
Review Comment:
Addressed in 9cd14699: is_safe_host now rejects any address where
ip.is_global is False (covers IPv4-mapped IPv6, translated, and special-use
forms) and keeps the explicit blocklist as defense-in-depth.
##########
superset/commands/dataset/importers/v1/utils.py:
##########
@@ -82,12 +84,20 @@ def get_dtype(df: pd.DataFrame, dataset: SqlaTable) ->
dict[str, VisitableType]:
def validate_data_uri(data_uri: str) -> None:
Review Comment:
Addressed in dfb304fa: load_data now uses a custom opener with
_ValidatingRedirectHandler, which calls validate_data_uri() on every redirect
target before following.
##########
superset/utils/network.py:
##########
@@ -14,14 +14,55 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import ipaddress
import platform
import socket
import subprocess
+# Networks that must never be reached via user-supplied hostnames.
+# Includes loopback, RFC-1918 private ranges, link-local (covers cloud
+# metadata endpoints such as 169.254.169.254), and IPv6 equivalents.
+_SSRF_UNSAFE_NETWORKS = (
+ ipaddress.ip_network("0.0.0.0/8"),
+ ipaddress.ip_network("10.0.0.0/8"),
+ ipaddress.ip_network("127.0.0.0/8"),
Review Comment:
Addressed in 9cd14699: 100.64.0.0/10 added to _SSRF_UNSAFE_NETWORKS.
##########
superset/utils/network.py:
##########
@@ -14,14 +14,55 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import ipaddress
import platform
import socket
import subprocess
+# Networks that must never be reached via user-supplied hostnames.
+# Includes loopback, RFC-1918 private ranges, link-local (covers cloud
+# metadata endpoints such as 169.254.169.254), and IPv6 equivalents.
+_SSRF_UNSAFE_NETWORKS = (
+ ipaddress.ip_network("0.0.0.0/8"),
+ ipaddress.ip_network("10.0.0.0/8"),
+ ipaddress.ip_network("127.0.0.0/8"),
+ ipaddress.ip_network("169.254.0.0/16"),
+ ipaddress.ip_network("172.16.0.0/12"),
+ ipaddress.ip_network("192.168.0.0/16"),
+ ipaddress.ip_network("::1/128"),
+ ipaddress.ip_network("fc00::/7"),
+ ipaddress.ip_network("fe80::/10"),
+)
+
PORT_TIMEOUT = 5
PING_TIMEOUT = 5
+def is_safe_host(host: str) -> bool:
+ """
+ Return True if ``host`` resolves exclusively to public, globally-routable
+ IP addresses.
+
+ Returns False if any resolved address falls within a private, loopback,
+ link-local, or otherwise non-routable range. An unresolvable host also
+ returns False.
+ """
+ try:
+ results = socket.getaddrinfo(host, None)
+ except socket.gaierror:
+ return False
+ if not results:
+ return False
+ for _, _, _, _, sockaddr in results:
+ try:
+ ip = ipaddress.ip_address(sockaddr[0])
+ except ValueError:
Review Comment:
Addressed in 04d042e5: IPv4-mapped IPv6 addresses are unwrapped to their
IPv4 form before unsafe-network checks.
##########
tests/unit_tests/utils/test_network.py:
##########
@@ -0,0 +1,99 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from unittest.mock import patch
+
+import pytest
+
+from superset.utils.network import is_safe_host
+
+
[email protected](
+ "resolved_ip, expected",
+ [
+ # Public IPs → safe
+ ("93.184.216.34", True), # example.com
Review Comment:
Addressed in de455257: parametrize argnames now use the tuple form.
##########
superset/commands/dataset/importers/v1/utils.py:
##########
@@ -82,12 +106,19 @@ def get_dtype(df: pd.DataFrame, dataset: SqlaTable) ->
dict[str, VisitableType]:
def validate_data_uri(data_uri: str) -> None:
"""
- Validate that the data URI is configured on DATASET_IMPORT_ALLOWED_URLS
- has a valid URL.
+ Validate that the data URI is permitted for dataset import.
+
+ Local ``file://`` URIs (used for bundled example data) are always allowed
+ since they do not make network requests. All other URIs must match a
+ pattern in ``DATASET_IMPORT_ALLOWED_DATA_URLS`` *and* resolve to a
+ publicly-routable host.
- :param data_uri:
- :return:
+ :param data_uri: the URI to validate
+ :raises DatasetForbiddenDataURI: if the URI is not permitted
"""
+ if data_uri.startswith("file://"):
+ return
Review Comment:
Addressed in 976afcf7 and 04d042e5: file:// URIs are now restricted to the
bundled examples folder via os.path.realpath comparison, and non-localhost
authority components are rejected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
