Explorar o código

Fixes #22894: Sanitize error message rendered during exception in CustomLinkColumn (#22895)

Jeremy Stretch hai 1 semana
pai
achega
a4dcd82606

+ 37 - 0
netbox/extras/tests/test_templatetags.py

@@ -1,6 +1,7 @@
 from django.contrib.auth import get_user_model
 from django.contrib.auth.context_processors import PermWrapper
 from django.test import RequestFactory, TestCase
+from django.utils.html import escape
 
 from core.models import ObjectType
 from dcim.models import Site
@@ -84,6 +85,12 @@ class CustomLinkRenderErrorEscapingTest(TestCase):
     XSS_NAME = '<img src=x onerror=alert(1)>'
     ESCAPED_NAME = '&lt;img src=x onerror=alert(1)&gt;'
 
+    # Subscripting a string with a nonexistent attribute yields an Undefined, and operating on it raises
+    # UndefinedError. These tests depend on Jinja2 quoting the subscript verbatim in that message (currently
+    # "'str object' has no attribute '<payload>'"); a change to Jinja2's message format would break them.
+    XSS_PAYLOAD = '" ></span><script>alert(1)</script>'
+    FAILING_TEMPLATE = f"{{{{ ''['{XSS_PAYLOAD}'] + 1 }}}}"
+
     @classmethod
     def setUpTestData(cls):
         cls.site = Site.objects.create(name='Site 1', slug='site-1')
@@ -134,3 +141,33 @@ class CustomLinkRenderErrorEscapingTest(TestCase):
         rendered = self.render(self.make_user_with_view_permission('user2'))
         self.assertNotIn(self.XSS_NAME, rendered)
         self.assertIn(self.ESCAPED_NAME, rendered)
+
+    def test_render_error_escapes_exception_message(self):
+        # The exception message reproduces the (attacker-controlled) template code, so it must be escaped
+        # in the error fallback as well (NB-3311).
+        custom_link = CustomLink.objects.create(
+            name='Custom Link 1',
+            enabled=True,
+            link_text=self.FAILING_TEMPLATE,
+            link_url='http://example.com/',
+        )
+        custom_link.object_types.set([ObjectType.objects.get_for_model(Site)])
+
+        rendered = self.render(self.make_user_with_view_permission('user3'))
+        self.assertNotIn(self.XSS_PAYLOAD, rendered)
+        self.assertIn(escape(self.XSS_PAYLOAD), rendered)
+
+    def test_render_error_escapes_grouped_exception_message(self):
+        # The grouped-link error fallback must likewise escape the exception message (NB-3311).
+        custom_link = CustomLink.objects.create(
+            name='Custom Link 1',
+            enabled=True,
+            group_name='Group 1',
+            link_text=self.FAILING_TEMPLATE,
+            link_url='http://example.com/',
+        )
+        custom_link.object_types.set([ObjectType.objects.get_for_model(Site)])
+
+        rendered = self.render(self.make_user_with_view_permission('user4'))
+        self.assertNotIn(self.XSS_PAYLOAD, rendered)
+        self.assertIn(escape(self.XSS_PAYLOAD), rendered)

+ 25 - 0
netbox/extras/tests/test_views.py

@@ -6,6 +6,7 @@ from django.contrib.contenttypes.models import ContentType
 from django.contrib.messages import get_messages
 from django.test import tag
 from django.urls import reverse
+from django.utils.html import escape
 
 from core.choices import JobStatusChoices, ManagedFileRootPathChoices
 from core.events import *
@@ -245,6 +246,30 @@ class CustomLinkRenderingTestCase(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertNotIn(f'FOO {site.name} BAR', str(response.content))
 
+    def test_list_view_custom_link_column_escapes_render_error(self):
+        # Jinja2 includes the invalid key verbatim in UndefinedError; this test intentionally depends on that format.
+        payload = '" ></span><script>alert(1)</script>'
+        customlink = CustomLink(
+            name='Test',
+            link_text=f"{{{{ ''['{payload}'] + 1 }}}}",
+            link_url='http://example.com/',
+            new_window=False
+        )
+        customlink.save()
+        customlink.object_types.set([ObjectType.objects.get_for_model(Site)])
+
+        site = Site(name='Test Site', slug='test-site')
+        site.save()
+
+        response = self.client.get(f"{reverse('dcim:site_list')}?include_columns=cl_Test")
+        self.assertEqual(response.status_code, 200)
+        content = response.content.decode()
+
+        # The error element must be present, but the payload must appear only in escaped form
+        self.assertIn('<span class="text-danger" title="', content)
+        self.assertNotIn(payload, content)
+        self.assertIn(escape(payload), content)
+
 
 class SavedFilterTestCase(ViewTestCases.PrimaryObjectViewTestCase):
     model = SavedFilter

+ 3 - 1
netbox/netbox/tables/columns.py

@@ -660,7 +660,9 @@ class CustomLinkColumn(tables.Column):
                 return mark_safe(f'<a href="{rendered["link"]}"{rendered["link_target"]}>{rendered["text"]}</a>')
         except Exception as e:
             error_text = _('Error')
-            return mark_safe(f'<span class="text-danger" title="{e}"><i class="mdi mdi-alert"></i> {error_text}</span>')
+            return format_html(
+                '<span class="text-danger" title="{}"><i class="mdi mdi-alert"></i> {}</span>', e, error_text
+            )
         return ''
 
     def value(self, record, table, **kwargs):