CVE-2022-4728: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: A vulnerability has been found in Graphite Web and classified as problematic. This vulnerability affects unknown code of the component Cookie Handler. The manipulation leads to cross site scripting. The attack can be initiated remotely. The exploit has been disclosed to the public and may be used. The name of the patch is 2f178f490e10efc03cd1d27c72f64ecab224eb23. It is recommended to apply a patch to fix this issue. VDB-216742 is the identifier assigned to this vulnerability.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2022-12-27T15:15Z
Repository: https://github.com/graphite-project/graphite-web
Language: Python
Files changed: webapp/content/js/dashboard.js, webapp/graphite/dashboard/views.py, webapp/graphite/errors.py, webapp/graphite/util.py, webapp/tests/base.py, webapp/tests/test_dashboard.py
Commit date: 2022-10-31 11:22:17 +0100
Commit message: Fix XSS in some dashboards queries (#2785)

* prevent xss on time range text fields

* prevent xss on dashbord queries
Diff: diff --git a/webapp/content/js/dashboard.js b/webapp/content/js/dashboard.js
index d5141605..e6ced3bc 100644
--- a/webapp/content/js/dashboard.js
+++ b/webapp/content/js/dashboard.js
@@ -161,10 +161,14 @@ function htmlEncode(input) {
   return input.replace(/[^a-zA-Z0-9 ]/g, function (chr) {
     return '&#' + chr.charCodeAt() + ';';
   });
 }
 
+function htmlStriped(input) {
+  return htmlEncode(input).replace(/\s/g, '_')
+}
+
 function initDashboard () {
 
   // Populate naming-scheme based datastructures
   Ext.each(schemes, function (schemeInfo) {
     schemeInfo.id = schemeInfo.name;
@@ -1227,11 +1231,11 @@ function timeRangeUpdated() {
 function selectRelativeTime() {
   var quantityField = new Ext.form.TextField({
     fieldLabel: 'Show the past',
     width: 90,
     allowBlank: false,
-    regex: /\d+/,
+    regex: /^\d+$/,
     regexText: 'Please enter a number',
     value: TimeRange.relativeStartQuantity
   });
 
   var unitField = new Ext.form.ComboBox({
@@ -1249,11 +1253,11 @@ function selectRelativeTime() {
   var untilQuantityField = new Ext.form.TextField({
     id: 'until-quantity-field',
     fieldLabel: 'Until',
     width: 90,
     allowBlank: true,
-    regex: /\d+/,
+    regex: /^\d+$/,
     regexText: 'Please enter a number',
     value: TimeRange.relativeUntilQuantity
   });
 
   var untilUnitField = new Ext.form.ComboBox({
@@ -1289,14 +1293,14 @@ function selectRelativeTime() {
 
   var win;
 
   function updateTimeRange() {
     TimeRange.type = 'relative';
-    TimeRange.relativeStartQuantity = quantityField.getValue();
-    TimeRange.relativeStartUnits = unitField.getValue();
-    TimeRange.relativeUntilQuantity = untilQuantityField.getValue();
-    TimeRange.relativeUntilUnits = untilUnitField.getValue();
+    TimeRange.relativeStartQuantity = htmlStriped(quantityField.getValue());
+    TimeRange.relativeStartUnits = htmlStriped(unitField.getValue());
+    TimeRange.relativeUntilQuantity = htmlStriped(untilQuantityField.getValue());
+    TimeRange.relativeUntilUnits = htmlStriped(untilUnitField.getValue());
     win.close();
     timeRangeUpdated();
   }
 
   win = new Ext.Window({
diff --git a/webapp/graphite/dashboard/views.py b/webapp/graphite/dashboard/views.py
index 5e5d6918..80750c7d 100644
--- a/webapp/graphite/dashboard/views.py
+++ b/webapp/graphite/dashboard/views.py
@@ -14,10 +14,11 @@ from graphite.compat import HttpResponse
 from graphite.dashboard.models import Dashboard, Template
 from graphite.dashboard.send_graph import send_graph_email
 from graphite.render.views import renderView
 from graphite.util import json
 from graphite.user_util import isAuthenticated
+from graphite.errors import handleInputParameterError, str_param
 
 fieldRegex = re.compile(r'<([^>]+)>')
 defaultScheme = {
   'name' : 'Everything',
   'pattern' : '<category>',
@@ -106,11 +107,13 @@ class DashboardConfig:
 
 
 config = DashboardConfig()
 
 
+@handleInputParameterError
 def dashboard(request, name=None):
+  name = str_param('name', name)
   dashboard_conf_missing = False
 
   try:
     config.check()
   except OSError as e:
@@ -153,11 +156,13 @@ def dashboard(request, name=None):
       context['initialState'] = dashboard.state
 
   return render(request, "dashboard.html", context)
 
 
+@handleInputParameterError
 def template(request, name, val):
+  name = str_param('name', name)
   template_conf_missing = False
 
   try:
     config.check()
   except OSError as e:
@@ -219,11 +224,14 @@ def getPermissions(user):
   if editGroup and len(user.groups.filter(name = editGroup)) == 0:
     permissions = []
   return permissions
 
 
+@handleInputParameterError
 def save(request, name):
+  name = str_param('name', name)
+
   if 'change' not in getPermissions(request.user):
     return json_response( dict(error="Must be logged in with appropriate permissions to save") )
   # Deserialize and reserialize as a validation step
   state = str( json.dumps( json.loads( request.POST['state'] ) ) )
 
@@ -236,11 +244,15 @@ def save(request, name):
     dashboard.save()
 
   return json_response( dict(success=True) )
 
 
+@handleInputParameterError
 def save_template(request, name, key):
+  name = str_param('name', name)
+  key = str_param('key', key)
+
   if 'change' not in getPermissions(request.user):
     return json_response( dict(error="Must be logged in with appropriate permissions to save the template") )
   # Deserialize and reserialize as a validation step
   state = str( json.dumps( json.loads( request.POST['state'] ) ) )
 
@@ -255,31 +267,37 @@ def save_template(request, name, key):
     template.save()
 
   return json_response( dict(success=True) )
 
 
+@handleInputParameterError
 def load(request, name):
+  name = str_param('name', name)
   try:
     dashboard = Dashboard.objects.get(name=name)
   except Dashboard.DoesNotExist:
     return json_response( dict(error="Dashboard '%s' does not exist. " % name) )
 
   return json_response( dict(state=json.loads(dashboard.state)) )
 
 
+@handleInputParameterError
 def load_template(request, name, val):
+  name = str_param('name', name)
   try:
     template = Template.objects.get(name=name)
   except Template.DoesNotExist:
     return json_response( dict(error="Template '%s' does not exist. " % name) )
 
   state = json.loads(template.loadState(val))
   state['name'] = '%s/%s' % (name, val)
   return json_response( dict(state=state) )
 
 
+@handleInputParameterError
 def delete(request, name):
+  name = str_param('name', name)
   if 'delete' not in getPermissions(request.user):
     return json_response( dict(error="Must be logged in with appropriate permissions to delete") )
 
   try:
     dashboard = Dashboard.objects.get(name=name)
@@ -288,11 +306,13 @@ def delete(request, name):
   else:
     dashboard.delete()
     return json_response( dict(success=True) )
 
 
+@handleInputParameterError
 def delete_template(request, name):
+  name = str_param('name', name)
   if 'delete' not in getPermissions(request.user):
     return json_response( dict(error="Must be logged in with appropriate permissions to delete the template") )
 
   try:
     template = Template.objects.get(name=name)
diff --git a/webapp/graphite/errors.py b/webapp/graphite/errors.py
index 14494630..a0144db0 100644
--- a/webapp/graphite/errors.py
+++ b/webapp/graphite/errors.py
@@ -1,7 +1,8 @@
 from django.http import HttpResponseBadRequest
 from graphite.logger import log
+from graphite.util import htmlEscape, is_unsafe_str
 
 
 class NormalizeEmptyResultError(Exception):
     # throw error for normalize() when empty
     pass
@@ -92,16 +93,26 @@ class InputParameterError(ValueError):
             msg += '; func: "{func}"'.format(func=func)
 
         return msg
 
 
-# Replace special characters "&", "<" and ">" to HTML-safe sequences.
-def escape(s):
-    s = s.replace("&", "&amp;")  # Must be done first!
-    s = s.replace("<", "&lt;")
-    s = s.replace(">", "&gt;")
+def safe_param(name, s):
+    if is_unsafe_str(s):
+        raise InputParameterError("{} contain unsafe symbols".format(name))
+    return s
+
+
+def is_unclean_str(s):
+    for symbol in '&<>~!@#$%^*()`':
+        if s.find(symbol) >= 0:
+            return True
+    return False
+
 
+def str_param(name, s):
+    if s is not None and is_unclean_str(s):
+        raise InputParameterError("{} contain restricted symbols".format(name))
     return s
 
 
 # decorator which turns InputParameterExceptions into Django's HttpResponseBadRequest
 def handleInputParameterError(f):
@@ -109,8 +120,8 @@ def handleInputParameterError(f):
         try:
             return f(*args, **kwargs)
         except InputParameterError as e:
             msgStr = str(e)
             log.warning('%s', msgStr)
-            return HttpResponseBadRequest(escape(msgStr))
+            return HttpResponseBadRequest(htmlEscape(msgStr))
 
     return new_f
diff --git a/webapp/graphite/util.py b/webapp/graphite/util.py
index 1b620331..1b588902 100644
--- a/webapp/graphite/util.py
+++ b/webapp/graphite/util.py
@@ -126,10 +126,25 @@ def find_escaped_pattern_fields(pattern_string):
   for index,part in enumerate(pattern_parts):
     if is_escaped_pattern(part):
       yield index
 
 
+# Replace special characters "&", "<" and ">" to HTML-safe sequences.
+def htmlEscape(s):
+    s = s.replace("&", "&amp;")  # Must be done first!
+    s = s.replace("<", "&lt;")
+    s = s.replace(">", "&gt;")
+    return s
+
+
+def is_unsafe_str(s):
+    for symbol in '<>':
+        if s.find(symbol) >= 0:
+            return True
+    return False
+
+
 def load_module(module_path, member=None):
   module_name = splitext(basename(module_path))[0]
   try:  # 'U' is default from Python 3.0 and deprecated since 3.9
     module_file = open(module_path, 'U')
   except ValueError:
diff --git a/webapp/tests/base.py b/webapp/tests/base.py
index 512e1e6d..844f28ea 100644
--- a/webapp/tests/base.py
+++ b/webapp/tests/base.py
@@ -1,9 +1,16 @@
 from django.test import TestCase as OriginalTestCase
 from graphite.worker_pool.pool import stop_pools
 
 
+def is_unsafe_str(s):
+    for symbol in '<>':
+        if s.find(symbol) > 0:
+            return True
+        return False
+
+
 class TestCase(OriginalTestCase):
     def tearDown(self):
         stop_pools()
 
     # Assert that a response is unsanitized (for check XSS issues)
@@ -13,7 +20,8 @@ class TestCase(OriginalTestCase):
                 response.status_code, status_code,
                 msg_prefix + "Couldn't retrieve content: Response code was %d"
                 " (expected %d)" % (response.status_code, status_code)
             )
 
-        xss = response.content.find(b"<") != -1 or response.content.find(b">") != -1
-        self.assertFalse(xss, msg=msg_prefix+str(response.content))
+        content = str(response.content)
+        xss = is_unsafe_str(content)
+        self.assertFalse(xss, msg=msg_prefix+content)
diff --git a/webapp/tests/test_dashboard.py b/webapp/tests/test_dashboard.py
index 160d0f94..4b2782f4 100644
--- a/webapp/tests/test_dashboard.py
+++ b/webapp/tests/test_dashboard.py
@@ -18,10 +18,12 @@ try:
     from django.contrib.auth import get_user_model
     User = get_user_model()
 except ImportError:
     from django.contrib.auth.models import User
 
+from graphite.util import htmlEscape
+
 
 class DashboardTest(TestCase):
     # Set config to the test config file
     settings.DASHBOARD_CONF = os.path.join(TEST_CONF_DIR, 'dashboard.conf')
 
@@ -242,27 +244,46 @@ class DashboardTest(TestCase):
         request = {"query": ""}
         response = self.client.get(url, request)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.content, b'{"templates": []}')
 
+    def test_dashboard_save_temporary_xss_name(self):
+        xssStr = htmlEscape('<img src=1 onerror=alert(1)>')
+        url = '/graphite/dashboard/save_template/' + xssStr + '/testkey'
+
+        request = copy.deepcopy(self.testtemplate)
+        response = self.client.post(url, request)
+        self.assertContains(response, 'name contain restricted symbols', status_code=400)
+
+    def test_dashboard_save_temporary_xss_key(self):
+        xssStr = htmlEscape('<img src=1 onerror=alert(1)>')
+        url = '/graphite/dashboard/save_template/testtemplate/' + xssStr
+
+        request = copy.deepcopy(self.testtemplate)
+        response = self.client.post(url, request)
+        self.assertContains(response, 'key contain restricted symbols', status_code=400)
+
     def test_dashboard_save_template(self):
         url = reverse('dashboard_save_template', args=['testtemplate', 'testkey'])
         request = copy.deepcopy(self.testtemplate)
         response = self.client.post(url, request)
         self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.content, b'{"success": true}')
 
-        # Save again after it now exists
+    # Save again after it now exists
     def test_dashboard_save_template_overwrite(self):
         url = reverse('dashboard_save_template', args=['testtemplate', 'testkey'])
         request = copy.deepcopy(self.testtemplate)
         response = self.client.post(url, request)
         self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.content, b'{"success": true}')
 
         url = reverse('dashboard_save_template', args=['testtemplate', 'testkey'])
         request = copy.deepcopy(self.testtemplate)
         response = self.client.post(url, request)
         self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.content, b'{"success": true}')
 
     def test_dashboard_find_template(self):
         url = reverse('dashboard_save_template', args=['testtemplate', 'testkey'])
         request = copy.deepcopy(self.testtemplate)
         response = self.client.post(url, request)

Vulnerable code:     regex: /\d+/,
    regex: /\d+/,
    TimeRange.relativeStartQuantity = quantityField.getValue();
    TimeRange.relativeStartUnits = unitField.getValue();
    TimeRange.relativeUntilQuantity = untilQuantityField.getValue();
    TimeRange.relativeUntilUnits = untilUnitField.getValue();
# Replace special characters "&", "<" and ">" to HTML-safe sequences.
def escape(s):
    s = s.replace("&", "&amp;")  # Must be done first!
    s = s.replace("<", "&lt;")
    s = s.replace(">", "&gt;")
            return HttpResponseBadRequest(escape(msgStr))
        xss = response.content.find(b"<") != -1 or response.content.find(b">") != -1
        self.assertFalse(xss, msg=msg_prefix+str(response.content))
        # Save again after it now exists
Fixed code: function htmlStriped(input) {
  return htmlEncode(input).replace(/\s/g, '_')
}

    regex: /^\d+$/,
    regex: /^\d+$/,
    TimeRange.relativeStartQuantity = htmlStriped(quantityField.getValue());
    TimeRange.relativeStartUnits = htmlStriped(unitField.getValue());
    TimeRange.relativeUntilQuantity = htmlStriped(untilQuantityField.getValue());
    TimeRange.relativeUntilUnits = htmlStriped(untilUnitField.getValue());
from graphite.errors import handleInputParameterError, str_param
@handleInputParameterError
  name = str_param('name', name)
@handleInputParameterError
  name = str_param('name', name)
@handleInputParameterError
  name = str_param('name', name)

@handleInputParameterError
  name = str_param('name', name)
  key = str_param('key', key)

@handleInputParameterError
  name = str_param('name', name)
@handleInputParameterError
  name = str_param('name', name)
@handleInputParameterError
  name = str_param('name', name)
@handleInputParameterError
  name = str_param('name', name)
from graphite.util import htmlEscape, is_unsafe_str
def safe_param(name, s):
    if is_unsafe_str(s):
        raise InputParameterError("{} contain unsafe symbols".format(name))
    return s


def is_unclean_str(s):
    for symbol in '&<>~!@#$%^*()`':
        if s.find(symbol) >= 0:
            return True
    return False

def str_param(name, s):
    if s is not None and is_unclean_str(s):
        raise InputParameterError("{} contain restricted symbols".format(name))
            return HttpResponseBadRequest(htmlEscape(msgStr))
# Replace special characters "&", "<" and ">" to HTML-safe sequences.
def htmlEscape(s):
    s = s.replace("&", "&amp;")  # Must be done first!
    s = s.replace("<", "&lt;")
    s = s.replace(">", "&gt;")
    return s


def is_unsafe_str(s):
    for symbol in '<>':
        if s.find(symbol) >= 0:
            return True
    return False


def is_unsafe_str(s):
    for symbol in '<>':
        if s.find(symbol) > 0:
            return True
        return False


        content = str(response.content)
        xss = is_unsafe_str(content)
        self.assertFalse(xss, msg=msg_prefix+content)
from graphite.util import htmlEscape

    def test_dashboard_save_temporary_xss_name(self):
        xssStr = htmlEscape('<img src=1 onerror=alert(1)>')
        url = '/graphite/dashboard/save_template/' + xssStr + '/testkey'

        request = copy.deepcopy(self.testtemplate)
        response = self.client.post(url, request)
        self.assertContains(response, 'name contain restricted symbols', status_code=400)

    def test_dashboard_save_temporary_xss_key(self):
        xssStr = htmlEscape('<img src=1 onerror=alert(1)>')
        url = '/graphite/dashboard/save_template/testtemplate/' + xssStr

        request = copy.deepcopy(self.testtemplate)
        response = self.client.post(url, request)
        self.assertContains(response, 'key contain restricted symbols', status_code=400)

        self.assertEqual(response.content, b'{"success": true}')
    # Save again after it now exists
        self.assertEqual(response.content, b'{"success": true}')
        self.assertEqual(response.content, b'{"success": true}')