You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by bo...@apache.org on 2017/03/28 23:40:40 UTC

incubator-airflow git commit: [AIRFLOW-1047] Sanitize strings passed to Markup

Repository: incubator-airflow
Updated Branches:
  refs/heads/master d8c0f59d5 -> fe9ebe3cc


[AIRFLOW-1047] Sanitize strings passed to Markup

We add the Apache-licensed bleach library and use
it to sanitize html
passed to Markup (which is supposed to be already
escaped). This avoids
some XSS issues with unsanitized user input being
displayed.

Closes #2193 from saguziel/aguziel-xss


Project: http://git-wip-us.apache.org/repos/asf/incubator-airflow/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-airflow/commit/fe9ebe3c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-airflow/tree/fe9ebe3c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-airflow/diff/fe9ebe3c

Branch: refs/heads/master
Commit: fe9ebe3ccf5fec4c491343659aa0c52e4125f66b
Parents: d8c0f59
Author: Alex Guziel <al...@airbnb.com>
Authored: Tue Mar 28 16:40:32 2017 -0700
Committer: Bolke de Bruin <bo...@xs4all.nl>
Committed: Tue Mar 28 16:40:32 2017 -0700

----------------------------------------------------------------------
 airflow/www/views.py        | 18 +++++++++++-------
 scripts/ci/requirements.txt |  1 +
 setup.py                    |  1 +
 tests/core.py               | 12 +++++++++++-
 4 files changed, 24 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/fe9ebe3c/airflow/www/views.py
----------------------------------------------------------------------
diff --git a/airflow/www/views.py b/airflow/www/views.py
index 15735b4..0def0a9 100644
--- a/airflow/www/views.py
+++ b/airflow/www/views.py
@@ -24,6 +24,7 @@ from datetime import datetime, timedelta
 import dateutil.parser
 import copy
 import json
+import bleach
 
 import inspect
 from textwrap import dedent
@@ -102,11 +103,12 @@ if conf.getboolean('webserver', 'FILTER_BY_OWNER'):
 
 
 def dag_link(v, c, m, p):
+    dag_id = bleach.clean(m.dag_id)
     url = url_for(
         'airflow.graph',
-        dag_id=m.dag_id)
+        dag_id=dag_id)
     return Markup(
-        '<a href="{url}">{m.dag_id}</a>'.format(**locals()))
+        '<a href="{}">{}</a>'.format(url, dag_id))
 
 
 def log_url_formatter(v, c, m, p):
@@ -117,20 +119,22 @@ def log_url_formatter(v, c, m, p):
 
 
 def task_instance_link(v, c, m, p):
+    dag_id = bleach.clean(m.dag_id)
+    task_id = bleach.clean(m.task_id)
     url = url_for(
         'airflow.task',
-        dag_id=m.dag_id,
-        task_id=m.task_id,
+        dag_id=dag_id,
+        task_id=task_id,
         execution_date=m.execution_date.isoformat())
     url_root = url_for(
         'airflow.graph',
-        dag_id=m.dag_id,
-        root=m.task_id,
+        dag_id=dag_id,
+        root=task_id,
         execution_date=m.execution_date.isoformat())
     return Markup(
         """
         <span style="white-space: nowrap;">
-        <a href="{url}">{m.task_id}</a>
+        <a href="{url}">{task_id}</a>
         <a href="{url_root}" title="Filter on this task and upstream">
         <span class="glyphicon glyphicon-filter" style="margin-left: 0px;"
             aria-hidden="true"></span>

http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/fe9ebe3c/scripts/ci/requirements.txt
----------------------------------------------------------------------
diff --git a/scripts/ci/requirements.txt b/scripts/ci/requirements.txt
index 3e307f7..7fdd18e 100644
--- a/scripts/ci/requirements.txt
+++ b/scripts/ci/requirements.txt
@@ -1,5 +1,6 @@
 alembic
 bcrypt
+bleach
 boto
 celery
 cgroupspy

http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/fe9ebe3c/setup.py
----------------------------------------------------------------------
diff --git a/setup.py b/setup.py
index 26a0e27..a582499 100644
--- a/setup.py
+++ b/setup.py
@@ -203,6 +203,7 @@ def do_setup():
         scripts=['airflow/bin/airflow'],
         install_requires=[
             'alembic>=0.8.3, <0.9',
+            'bleach==2.0.0',
             'configparser>=3.5.0, <3.6.0',
             'croniter>=0.3.8, <0.4',
             'dill>=0.2.2, <0.3',

http://git-wip-us.apache.org/repos/asf/incubator-airflow/blob/fe9ebe3c/tests/core.py
----------------------------------------------------------------------
diff --git a/tests/core.py b/tests/core.py
index 848553a..18dcb4d 100644
--- a/tests/core.py
+++ b/tests/core.py
@@ -14,6 +14,7 @@
 
 from __future__ import print_function
 
+import bleach
 import doctest
 import json
 import os
@@ -1423,7 +1424,7 @@ class CliTests(unittest.TestCase):
         os.remove('variables1.json')
         os.remove('variables2.json')
 
-class CSRFTests(unittest.TestCase):
+class SecurityTests(unittest.TestCase):
     def setUp(self):
         configuration.load_test_config()
         configuration.conf.set("webserver", "authenticate", "False")
@@ -1458,6 +1459,15 @@ class CSRFTests(unittest.TestCase):
         response = self.app.post("/admin/queryview/", data=dict(csrf_token=csrf))
         self.assertEqual(200, response.status_code)
 
+    def test_xss(self):
+        try:
+            self.app.get("/admin/airflow/tree?dag_id=<script>alert(123456)</script>")
+        except:
+            # exception is expected here since dag doesnt exist
+            pass
+        response = self.app.get("/admin/log", follow_redirects=True)
+        self.assertIn(bleach.clean("<script>alert(123456)</script>"), response.data.decode('UTF-8'))
+
     def tearDown(self):
         configuration.conf.set("webserver", "expose_config", "False")
         self.dag_bash.clear(start_date=DEFAULT_DATE, end_date=datetime.now())