Skip to content

Commit

Permalink
Simplify origin string cleaning (apache#27143)
Browse files Browse the repository at this point in the history
(cherry picked from commit 68cb2da)
  • Loading branch information
jedcunningham committed Nov 3, 2022
1 parent b0c21b3 commit dd94f33
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 16 deletions.
20 changes: 7 additions & 13 deletions airflow/www/views.py
Expand Up @@ -30,7 +30,7 @@
from json import JSONDecodeError
from operator import itemgetter
from typing import Callable, Iterable, List, Optional, Set, Tuple, Union
from urllib.parse import parse_qsl, unquote, urlencode, urlparse
from urllib.parse import unquote, urljoin, urlsplit

import lazy_object_proxy
import nvd3
Expand Down Expand Up @@ -140,27 +140,21 @@ def truncate_task_duration(task_duration):

def get_safe_url(url):
"""Given a user-supplied URL, ensure it points to our web server"""
valid_schemes = ['http', 'https', '']
valid_netlocs = [request.host, '']

if not url:
return url_for('Airflow.index')

parsed = urlparse(url)

# If the url contains semicolon, redirect it to homepage to avoid
# potential XSS. (Similar to https://github.com/python/cpython/pull/24297/files (bpo-42967))
if ';' in unquote(url):
return url_for('Airflow.index')

query = parse_qsl(parsed.query, keep_blank_values=True)

url = parsed._replace(query=urlencode(query)).geturl()

if parsed.scheme in valid_schemes and parsed.netloc in valid_netlocs:
return url
host_url = urlsplit(request.host_url)
redirect_url = urlsplit(urljoin(request.host_url, url))
if not (redirect_url.scheme in ("http", "https") and host_url.netloc == redirect_url.netloc):
return url_for('Airflow.index')

return url_for('Airflow.index')
# This will ensure we only redirect to the right scheme/netloc
return redirect_url.geturl()


def get_date_time_num_runs_dag_runs_form_data(www_request, session, dag):
Expand Down
6 changes: 6 additions & 0 deletions tests/www/views/test_views.py
Expand Up @@ -116,7 +116,13 @@ def test_task_start_date_filter(admin_client, url, content):
"test_url, expected_url",
[
("", "/home"),
("javascript:alert(1)", "/home"),
(" javascript:alert(1)", "http://localhost:8080/ javascript:alert(1)"),
("http://google.com", "/home"),
("google.com", "http://localhost:8080/google.com"),
("\\/google.com", "http://localhost:8080/\\/google.com"),
("//google.com", "/home"),
("\\/\\/google.com", "http://localhost:8080/\\/\\/google.com"),
("36539'%3balert(1)%2f%2f166", "/home"),
(
"http://localhost:8080/trigger?dag_id=test&origin=36539%27%3balert(1)%2f%2f166&abc=2",
Expand Down
6 changes: 3 additions & 3 deletions tests/www/views/test_views_trigger_dag.py
Expand Up @@ -103,14 +103,14 @@ def test_trigger_dag_form(admin_client):
("36539'%3balert(1)%2f%2f166", "/home"),
(
'"><script>alert(99)</script><a href="',
"&#34;&gt;&lt;script&gt;alert(99)&lt;/script&gt;&lt;a href=&#34;",
"http://localhost/&#34;&gt;&lt;script&gt;alert(99)&lt;/script&gt;&lt;a href=&#34;",
),
(
"%2Ftree%3Fdag_id%3Dexample_bash_operator';alert(33)//",
"/home",
),
("%2Ftree%3Fdag_id%3Dexample_bash_operator", "/tree?dag_id=example_bash_operator"),
("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"),
("%2Ftree%3Fdag_id%3Dexample_bash_operator", "http://localhost/tree?dag_id=example_bash_operator"),
("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "http://localhost/graph?dag_id=example_bash_operator"),
],
)
def test_trigger_dag_form_origin_url(admin_client, test_origin, expected_origin):
Expand Down

0 comments on commit dd94f33

Please sign in to comment.