Skip to content

Commit 2020051

Browse files
potiukCloud Composer Team
authored and
Cloud Composer Team
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. GitOrigin-RevId: 87c5c9fa629317090ce65ec4c686596a2c4cd148
1 parent 515b213 commit 2020051

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
@@ -153,6 +153,27 @@ def sanitize_args(args: dict[str, str]) -> dict[str, str]:
153153
return {key: value for key, value in args.items() if not key.startswith("_")}
154154

155155

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

187+
url = url.lstrip(_WHATWG_C0_CONTROL_OR_SPACE)
188+
166189
host_url = urlsplit(request.host_url)
167190
redirect_url = urlsplit(urljoin(request.host_url, url))
168191
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
@@ -193,7 +193,7 @@ def test_task_dag_id_equals_filter(admin_client, url, content):
193193
[
194194
("", "/home"),
195195
("javascript:alert(1)", "/home"),
196-
(" javascript:alert(1)", "http://localhost:8080/ javascript:alert(1)"),
196+
(" javascript:alert(1)", "/home"),
197197
("http://google.com", "/home"),
198198
("google.com", "http://localhost:8080/google.com"),
199199
("\\/google.com", "http://localhost:8080/\\/google.com"),

0 commit comments

Comments
 (0)