[fix] Make sure to HTML escape all dashboard labels

(cherry picked from commit 746b146)

Author: nemesifier

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
@@ -126,7 +127,12 @@ def get_dashboard_context(request):
         group_by = query_params.get('group_by')
         annotate = query_params.get('annotate')
         aggregate = query_params.get('aggregate')
+
         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)
@@ -175,9 +181,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

@@ -81,7 +81,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

@@ -11,9 +11,11 @@
     unregister_dashboard_chart,
     unregister_dashboard_template,
 )
+from openwisp_utils.admin_theme.dashboard import get_dashboard_context
 
-from ..models import Project
+from ..models import Operator, Project
 from . import AdminTestMixin
+from .utils import MockRequest, MockUser
 
 
 class TestDashboardSchema(UnitTestCase):
@@ -178,3 +180,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;',
+        )

tests/test_project/tests/utils.py

@@ -1,8 +1,10 @@
 import json
 import os
+import uuid
 
 from django.conf import settings
 from django.contrib.auth import get_user_model
+from django.contrib.auth.models import AnonymousUser
 from selenium import webdriver
 from selenium.common.exceptions import NoSuchElementException
 from selenium.webdriver.common.by import By
@@ -267,3 +269,17 @@ def wait_for_dropdown(self, filter_class):
                 (By.CSS_SELECTOR, f'.{filter_class} .filter-options')
             )
         )
+
+
+class MockUser:
+    def __init__(self, is_superuser=False):
+        self.is_superuser = is_superuser
+        self.id = uuid.uuid4()
+
+
+class MockRequest:
+    def __init__(self, user=None):
+        if user:
+            self.user = user
+        else:
+            self.user = AnonymousUser()