[fix] Make sure to HTML escape all dashboard labels

openwisp_utils/admin_theme/dashboard.py

@@ -1,4 +1,5 @@
 import copy
+import html
 
 from django.core.exceptions import ImproperlyConfigured
 from django.db.models import Count
@@ -145,7 +146,12 @@ def get_dashboard_context(request):
         aggregate = query_params.get('aggregate')
         org_field = query_params.get('organization_field')
         default_org_field = 'organization_id'
+
         labels_i18n = value.get('labels')
+        # HTML escape labels defined in configuration to prevent breaking the JS
+        if labels_i18n:
+            for label_key, label_value in labels_i18n.items():
+                labels_i18n[label_key] = html.escape(label_value)
 
         try:
             model = load_model(app_label, model_name)
@@ -216,9 +222,13 @@ def get_dashboard_context(request):
                 if labels_i18n and qs_key in labels_i18n:
                     # store original label as filter, but only
                     # if we have more than the empty default label defined
-                    # if len(labels_i18n.keys()) > 1
                     filters.append(label)
                     label = labels_i18n[qs_key]
+                else:
+                    # HTML escape labels coming from values in the DB
+                    # to avoid possible XSS attacks caused by
+                    # malicious DB values set by users
+                    label = html.escape(label)
                 labels.append(label)
                 # use predefined colors if available,
                 # otherwise the JS lib will choose automatically

tests/test_project/apps.py

@@ -89,7 +89,9 @@ def register_dashboard_charts(self):
                     'without_operator__sum': '#353c44',
                 },
                 'labels': {
-                    'with_operator__sum': _('Projects with operators'),
+                    # the <strong> is for testing purposes to
+                    # verify it's being HTML escaped correctly
+                    'with_operator__sum': _('<strong>Projects with operators</strong>'),
                     'without_operator__sum': _('Projects without operators'),
                 },
                 'filters': {

tests/test_project/tests/test_dashboard.py

@@ -15,7 +15,7 @@
 )
 from openwisp_utils.admin_theme.dashboard import get_dashboard_context
 
-from ..models import Project, RadiusAccounting
+from ..models import Operator, Project, RadiusAccounting
 from . import AdminTestMixin, CreateMixin
 from .utils import MockRequest, MockUser
 
@@ -271,3 +271,26 @@ def test_dashboard_disabled(self):
         with self.subTest('Test "Dashboard" is absent from menu items'):
             response = self.client.get(reverse('admin:index'))
             self.assertNotContains(response, 'Dashboard')
+
+    def test_get_dashboard_context_html_escape(self):
+        # craft malicious DB value which will be shown in labels
+        project = Project.objects.create(name='<script>alert(1)</script>')
+        Operator.objects.create(project=project, first_name='xss', last_name='xss')
+        # prepare mock request and get context
+        mocked_user = MockUser(is_superuser=True)
+        mocked_request = MockRequest(user=mocked_user)
+        context = get_dashboard_context(mocked_request)
+        # ensure DB value is escaped
+        self.assertEqual(
+            context['dashboard_charts'][0]['query_params']['labels'][0],
+            '&lt;script&gt;alert(1)&lt;/script&gt;',
+        )
+        # ensure configured labels are escaped
+        self.assertEqual(
+            context['dashboard_charts'][1]['labels']['with_operator__sum'],
+            '&lt;strong&gt;Projects with operators&lt;/strong&gt;',
+        )
+        self.assertEqual(
+            context['dashboard_charts'][1]['query_params']['labels'][0],
+            '&lt;strong&gt;Projects with operators&lt;/strong&gt;',
+        )