Skip to content

Commit 5b41ed8

Browse files
potiukElad Kalif
authored and
Elad Kalif
committed
Fix failing get_safe_url tests for latest Python 3.8 and 3.9 (#31766)
The latest release of Python 3.8 and 3.9 have been just released that contain the fix to a security vulnerability backported to those versions: python/cpython#102153 Release notes: * https://www.python.org/downloads/release/python-3817/ * https://www.python.org/downloads/release/python-3917/ The fix improved sanitizing of the URLs and until Python 3.10 and 3.11 get released, we need to add the sanitization ourselves to pass tests on all versions. In order to improve security of airflow users and make the tests work regardless whether the users have latest Python versions released, we add extra sanitisation step to the URL to apply the standard WHATWG specification. (cherry picked from commit 87c5c9f)
1 parent cc3cd20 commit 5b41ed8

File tree

2 files changed

+24
-1
lines changed

2 files changed

+24
-1
lines changed

airflow/www/views.py

+23
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,27 @@ def sanitize_args(args: dict[str, str]) -> dict[str, str]:
162162
return {key: value for key, value in args.items() if not key.startswith("_")}
163163

164164

165+
# Following the release of https://github.com/python/cpython/issues/102153 in Python 3.8.17 and 3.9.17 on
166+
# June 6, 2023, we are adding extra sanitization of the urls passed to get_safe_url method to make it works
167+
# the same way regardless if the user uses latest Python patchlevel versions or not. This also follows
168+
# a recommended solution by the Python core team.
169+
#
170+
# From: https://github.com/python/cpython/commit/d28bafa2d3e424b6fdcfd7ae7cde8e71d7177369
171+
#
172+
# We recommend that users of these APIs where the values may be used anywhere
173+
# with security implications code defensively. Do some verification within your
174+
# code before trusting a returned component part. Does that ``scheme`` make
175+
# sense? Is that a sensible ``path``? Is there anything strange about that
176+
# ``hostname``? etc.
177+
#
178+
# C0 control and space to be stripped per WHATWG spec.
179+
# == "".join([chr(i) for i in range(0, 0x20 + 1)])
180+
_WHATWG_C0_CONTROL_OR_SPACE = (
181+
"\x00\x01\x02\x03\x04\x05\x06\x07\x08\t\n\x0b\x0c"
182+
"\r\x0e\x0f\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f "
183+
)
184+
185+
165186
def get_safe_url(url):
166187
"""Given a user-supplied URL, ensure it points to our web server"""
167188
if not url:
@@ -172,6 +193,8 @@ def get_safe_url(url):
172193
if ";" in unquote(url):
173194
return url_for("Airflow.index")
174195

196+
url = url.lstrip(_WHATWG_C0_CONTROL_OR_SPACE)
197+
175198
host_url = urlsplit(request.host_url)
176199
redirect_url = urlsplit(urljoin(request.host_url, url))
177200
if not (redirect_url.scheme in ("http", "https") and host_url.netloc == redirect_url.netloc):

tests/www/views/test_views.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ def test_task_dag_id_equals_filter(admin_client, url, content):
181181
[
182182
("", "/home"),
183183
("javascript:alert(1)", "/home"),
184-
(" javascript:alert(1)", "http://localhost:8080/ javascript:alert(1)"),
184+
(" javascript:alert(1)", "/home"),
185185
("http://google.com", "/home"),
186186
("google.com", "http://localhost:8080/google.com"),
187187
("\\/google.com", "http://localhost:8080/\\/google.com"),

0 commit comments

Comments
 (0)