Explorar el Código

[beeswax] Fix HTML escaping of result table

Check for XSS and NULL display.
Move logic to backend to do it only once and more efficiently.
Romain Rigaux hace 11 años
padre
commit
4b5c9afea4

+ 1 - 13
apps/beeswax/src/beeswax/templates/execute.mako

@@ -1474,19 +1474,7 @@ function resultsTable(e, data) {
       "fnDrawCallback": function (oSettings) {
         reinitializeTable();
       },
-      "fnRowCallback": function (nRow, aData, iDisplayIndex, iDisplayIndexFull) {
-        // Make sure null values are seen as NULL and are escaped.
-        var tmpDiv = $('<div />');
-        for (var j = 0; j < aData.length; ++j) {
-          var cell = $(nRow).find('td:eq(' + j + ')');
-          if (aData[j] == null) {
-            cell.html("NULL");
-          } else {
-            cell.html(tmpDiv.text(cell.html()).html());
-          }
-        }
-        return nRow;
-      },
+
       "aoColumnDefs": [
         {
           "sType": "numeric",

+ 20 - 1
apps/beeswax/src/beeswax/tests.py

@@ -34,6 +34,7 @@ from nose.tools import assert_true, assert_equal, assert_false, assert_not_equal
 from nose.plugins.skip import SkipTest
 
 from django.utils.encoding import smart_str
+from django.utils.html import escape
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 
@@ -78,7 +79,7 @@ def _make_query(client, query, submission_type="Execute",
 
   # Should be in the history if it's submitted.
   if submission_type == 'Execute':
-    fragment = collapse_whitespace(smart_str(query[:20]))
+    fragment = collapse_whitespace(smart_str(escape(query[:20])))
     verify_history(client, fragment=fragment)
 
   return res
@@ -219,6 +220,24 @@ for x in sys.stdin:
     # Header line plus data lines...
     assert_equal(257, response.content.count("\n"))
 
+  def test_result_escaping(self):
+    # Check for XSS and NULL display
+    QUERY = """
+      SELECT 'abc', 1.0, 1=1, 1, 1/0, '<a>lala</a>lulu' from test LIMIT 3;
+    """
+    response = _make_query(self.client, QUERY, local=False)
+    content = json.loads(response.content)
+    assert_true('watch_url' in content)
+
+    response = wait_for_query_to_finish(self.client, response, max=180.0)
+    content = fetch_query_result_data(self.client, response)
+
+    assert_equal([
+        [u'abc', 1.0, True, 1, u'NULL', u'&lt;a&gt;lala&lt;/a&gt;lulu'],
+        [u'abc', 1.0, True, 1, u'NULL', u'&lt;a&gt;lala&lt;/a&gt;lulu'],
+        [u'abc', 1.0, True, 1, u'NULL', u'&lt;a&gt;lala&lt;/a&gt;lulu'],
+      ], content["results"], content)
+
   def test_query_with_udf(self):
     """
     Testing query with udf

+ 16 - 1
apps/beeswax/src/beeswax/views.py

@@ -25,6 +25,7 @@ from django.contrib.auth.models import User
 from django.db.models import Q
 from django.http import HttpResponse, QueryDict
 from django.shortcuts import redirect
+from django.utils.html import escape
 from django.utils.translation import ugettext as _
 from django.core.urlresolvers import reverse
 
@@ -425,7 +426,21 @@ def view_results(request, id, first_row=0):
       downloadable = False
     else:
       results = db.fetch(handle, start_over, 100)
-      data = list(results.rows()) # Materialize results
+      data = []
+      
+      # Materialize and HTML escape results
+      # TODO: use Number + list comprehension
+      for row in results.rows():
+        escaped_row = []
+        for field in row:          
+          if isinstance(field, (int, long, float, complex, bool)):
+            escaped_field = field
+          elif field is None:
+            escaped_field = 'NULL'
+          else:
+            escaped_field = escape(field)              
+          escaped_row.append(escaped_field)
+        data.append(escaped_row)
 
       # We display the "Download" button only when we know that there are results:
       downloadable = first_row > 0 or data