Merge pull request #2907 from graphite-project/copilot/fix-issue-2870

Fix reflected XSS in /metrics/find via `from`/`until` parameters

Author: deniszh

webapp/tests/base.py

@@ -4,9 +4,9 @@
 
 def is_unsafe_str(s):
     for symbol in '<>':
-        if s.find(symbol) > 0:
+        if s.find(symbol) >= 0:
             return True
-        return False
+    return False
 
 
 class TestCase(OriginalTestCase):

webapp/tests/test_xss.py

@@ -29,7 +29,7 @@ def test_render_xss(self):
         xssStr = '<noscript><p title="</noscript><img src=x onerror=alert() onmouseover=alert()>">'
 
         # Check for issue #2779 and others
-        response = self.client.get(url, {'target': 'test', 'format': 'raw', 'cacheTimeout': xssStr, 'from': xssStr})
+        response = self.client.get(url, {'target': 'test', 'format': 'raw', 'cacheTimeout': xssStr, 'from': xssStr, 'until': xssStr})
         self.assertXSS(response, status_code=400, msg_prefix='XSS detected: ')
 
 
@@ -38,5 +38,14 @@ def test_render_xss(self):
         url = reverse('metrics_find')
         xssStr = '<noscript><p title="</noscript><img src=x onerror=alert() onmouseover=alert()>">'
 
-        response = self.client.get(url, {'query': 'test', 'local': xssStr, 'from': xssStr, 'tz': xssStr})
+        response = self.client.get(url, {'query': 'test', 'local': xssStr, 'from': xssStr, 'until': xssStr, 'tz': xssStr})
         self.assertXSS(response, status_code=400, msg_prefix='XSS detected: ')
+
+    def test_find_xss_script_tag(self):
+        """Test that <script> tags in from/until parameters are properly escaped (issue #2870)"""
+        url = reverse('metrics_find')
+        xssStr = "<script>alert('XSS')</script>"
+
+        for param in ('from', 'until'):
+            response = self.client.get(url, {'query': 'test', param: xssStr})
+            self.assertXSS(response, status_code=400, msg_prefix='XSS detected in %s: ' % param)