Просмотр исходного кода

fix(dcim): Restrict rack elevation highlight to exact id/name matches (#23299)

The highlight parameter on the rack elevation SVG passed caller-supplied
field paths and lookups directly into a Q object. Because the permission
restriction only limits which devices can match, a predicate traversing
related objects (e.g. tenant__description__startswith) could be used to
infer data the user is not permitted to view.

Accept only the id and name attributes with exact matching, and ignore
values that fail field validation. Also skip highlight params lacking a
colon, which previously raised an unhandled ValueError.

Fixes #23198
Jeremy Stretch 1 день назад
Родитель
Сommit
5d4a996c70
3 измененных файлов с 48 добавлено и 7 удалено
  1. 1 3
      netbox/dcim/api/views.py
  2. 12 4
      netbox/dcim/svg/racks.py
  3. 35 0
      netbox/dcim/tests/test_api.py

+ 1 - 3
netbox/dcim/api/views.py

@@ -215,10 +215,8 @@ class RackViewSet(NetBoxModelViewSet):
             # Determine attributes for highlighting devices (if any)
             highlight_params = []
             for param in request.GET.getlist('highlight'):
-                try:
+                if ':' in param:
                     highlight_params.append(param.split(':', 1))
-                except ValueError:
-                    pass
 
             # Render and return the elevation as an SVG drawing with the correct content type
             drawing = rack.get_elevation_svg(

+ 12 - 4
netbox/dcim/svg/racks.py

@@ -2,7 +2,8 @@ import decimal
 
 import svgwrite
 from django.conf import settings
-from django.core.exceptions import FieldError
+from django.core.exceptions import ValidationError
+from django.core.validators import ProhibitNullCharactersValidator
 from django.db.models import Q
 from django.template.defaultfilters import floatformat
 from django.urls import reverse
@@ -119,11 +120,18 @@ class RackElevationSVG:
         if highlight_params:
             q = Q()
             for k, v in highlight_params:
+                # Ignore any unsupported attributes (including related fields & lookups)
+                if k not in ('id', 'name'):
+                    continue
+                try:
+                    # Validate the value against the field before including it
+                    ProhibitNullCharactersValidator()(v)
+                    permitted_devices.model._meta.get_field(k).get_prep_value(v)
+                except (TypeError, ValueError, ValidationError):
+                    continue
                 q |= Q(**{k: v})
-            try:
+            if q:
                 self.highlight_devices = permitted_devices.filter(q)
-            except FieldError:
-                pass
 
     @staticmethod
     def _add_gradient(drawing, id_, color):

+ 35 - 0
netbox/dcim/tests/test_api.py

@@ -1375,6 +1375,41 @@ class RackTestCase(APIViewTestCases.APIViewTestCase):
         self.assertHttpStatus(response, status.HTTP_200_OK)
         self.assertEqual(response.get('Content-Type'), 'image/svg+xml')
 
+    def test_get_rack_elevation_svg_highlight(self):
+        """
+        Highlighting devices in an SVG rack elevation supports only exact matches on permitted fields.
+        """
+        rack = Rack.objects.first()
+        tenant = Tenant.objects.create(name='Tenant 1', slug='tenant-1', description='SECRET')
+        device1 = create_test_device('Device 1', site=rack.site, rack=rack, position=1, face='front', tenant=tenant)
+        create_test_device('Device 2', site=rack.site, rack=rack, position=10, face='front')
+        self.add_permissions('dcim.view_rack', 'dcim.view_device')
+        url = reverse('dcim-api:rack-elevation', kwargs={'pk': rack.pk})
+
+        def is_highlighted(*params):
+            query = '&'.join(f'highlight={p}' for p in params)
+            response = self.client.get(f'{url}?render=svg&{query}', **self.header)
+            self.assertHttpStatus(response, status.HTTP_200_OK)
+            return 'slot shaded' in response.content.decode()
+
+        # Supported attributes
+        self.assertTrue(is_highlighted(f'id:{device1.pk}'))
+        self.assertTrue(is_highlighted('name:Device 1'))
+        self.assertFalse(is_highlighted('name:Nonexistent'))
+
+        # Related fields and lookup expressions must be ignored
+        self.assertFalse(is_highlighted('tenant__description__startswith:S'))
+        self.assertFalse(is_highlighted('tenant__description:SECRET'))
+        self.assertFalse(is_highlighted('name__startswith:Device 1'))
+        self.assertFalse(is_highlighted(f'tenant_id:{tenant.pk}'))
+
+        # Malformed and invalid values must be ignored
+        self.assertFalse(is_highlighted('id'))
+        self.assertFalse(is_highlighted('id:foo'))
+        self.assertFalse(is_highlighted('name:%00'))
+        self.assertTrue(is_highlighted('id:foo', 'name:Device 1'))
+        self.assertTrue(is_highlighted('name:%00', 'name:Device 1'))
+
 
 class RackReservationTestCase(APIViewTestCases.APIViewTestCase):
     model = RackReservation