This is an automated email from the ASF dual-hosted git repository. kaxilnaik pushed a commit to branch origin-fix in repository https://gitbox.apache.org/repos/asf/airflow.git
commit d6a1ae1a7ddb1bb67f1d35dc8a72d387575176a1 Author: Kaxil Naik <[email protected]> AuthorDate: Wed Nov 18 19:52:14 2020 +0000 Webserver: Further Sanitize values passed to origin param Follow-up of https://github.com/apache/airflow/pull/10334 --- airflow/www/views.py | 7 ++++++- tests/www/test_views.py | 26 +++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index fc2d2d0..bc4ac40 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -28,7 +28,7 @@ from collections import defaultdict from datetime import datetime, timedelta from json import JSONDecodeError from typing import Dict, List, Optional, Tuple -from urllib.parse import unquote, urlparse +from urllib.parse import parse_qsl, unquote, urlencode, urlparse import lazy_object_proxy import nvd3 @@ -108,8 +108,13 @@ def get_safe_url(url): valid_schemes = ['http', 'https', ''] valid_netlocs = [request.host, ''] + # Remove single quotes + url = url.replace("'", "") parsed = urlparse(url) + query = parse_qsl(parsed.query) + url = parsed._replace(query=urlencode(query)).geturl() + if parsed.scheme in valid_schemes and parsed.netloc in valid_netlocs: return url diff --git a/tests/www/test_views.py b/tests/www/test_views.py index c3e9963..39fe6d8 100644 --- a/tests/www/test_views.py +++ b/tests/www/test_views.py @@ -62,7 +62,7 @@ from airflow.utils.state import State from airflow.utils.timezone import datetime from airflow.utils.types import DagRunType from airflow.www import app as application -from airflow.www.views import ConnectionModelView +from airflow.www.views import ConnectionModelView, get_safe_url from tests.test_utils import fab_utils from tests.test_utils.asserts import assert_queries_count from tests.test_utils.config import conf_vars @@ -2772,6 +2772,7 @@ class TestTriggerDag(TestBase): [ ("javascript:alert(1)", "/home"), ("http://google.com";, "/home"), + ("%2Ftree%3Fdag_id%3Dexample_bash_operator%27;alert(33)//", "/tree?dag_id=example_bash_operator"), ("%2Ftree%3Fdag_id%3Dexample_bash_operator", "/tree?dag_id=example_bash_operator"), ("%2Fgraph%3Fdag_id%3Dexample_bash_operator", "/graph?dag_id=example_bash_operator"), ] @@ -3293,3 +3294,26 @@ class TestDecorators(TestBase): self.check_last_log( "example_bash_operator", event="clear", execution_date=self.EXAMPLE_DAG_DEFAULT_DATE ) + + +class TestHelperFunctions(unittest.TestCase): + @parameterized.expand( + [ + # ("javascript:alert(1)", "/home"), + ("http://google.com";, "/home"), + ( + "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag';alert(33)//", + "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%25test_dag";, + ), + ( + "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%test_dag";, + "http://localhost:8080/trigger?dag_id=test_dag&origin=%2Ftree%3Fdag_id%25test_dag";, + ), + ] + ) + @mock.patch("airflow.www.views.url_for") + @mock.patch("airflow.www.views.request") + def test_get_safe_url(self, test_url, expected_url, mock_req, mock_url_for): + mock_req.host = 'localhost:8080' + mock_url_for.return_value = "/home" + self.assertEqual(get_safe_url(test_url), expected_url)
