Fixes #22399: Enforce object permissions for relevant static media (#22400)

netbox/netbox/tests/test_views.py

@@ -1,9 +1,13 @@
 import urllib.parse
+from unittest.mock import patch
 
+from django.contrib.contenttypes.models import ContentType
+from django.http import HttpResponse
 from django.test import Client, override_settings
 from django.urls import reverse
 
-from dcim.models import Site
+from dcim.models import DeviceType, Manufacturer, Site
+from extras.models import ImageAttachment
 from netbox.constants import EMPTY_TABLE_TEXT
 from netbox.search.backends import search_backend
 from utilities.testing import TestCase
@@ -78,6 +82,27 @@ class SearchViewTestCase(TestCase):
 
 class MediaViewTestCase(TestCase):
 
+    @classmethod
+    def setUpTestData(cls):
+        site = Site.objects.create(name='Site 1', slug='site-1')
+        ct = ContentType.objects.get_for_model(Site)
+        cls.image_attachment = ImageAttachment.objects.create(
+            object_type=ct,
+            object_id=site.pk,
+            name='Test Image',
+            image='image-attachments/site_1_test.jpg',
+            image_height=100,
+            image_width=100,
+        )
+
+        manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
+        cls.device_type = DeviceType.objects.create(
+            model='Device Type 1',
+            slug='device-type-1',
+            manufacturer=manufacturer,
+            front_image='devicetype-images/front.jpg',
+        )
+
     def test_media_login_required(self):
         url = reverse('media', kwargs={'path': 'foo.txt'})
         response = Client().get(url)
@@ -92,3 +117,45 @@ class MediaViewTestCase(TestCase):
 
         # Unauthenticated request should return a 404 (not found)
         self.assertHttpStatus(response, 404)
+
+    def test_image_attachment_with_permission(self):
+        self.add_permissions('extras.view_imageattachment')
+        url = reverse('media', kwargs={'path': self.image_attachment.image.name})
+        with patch('netbox.views.misc.serve', return_value=HttpResponse(status=200)):
+            response = self.client.get(url)
+        self.assertHttpStatus(response, 200)
+        self.assertEqual(response['Content-Disposition'], 'attachment')
+        self.assertEqual(response['X-Content-Type-Options'], 'nosniff')
+
+    def test_image_attachment_without_permission(self):
+        url = reverse('media', kwargs={'path': self.image_attachment.image.name})
+        response = self.client.get(url)
+        self.assertHttpStatus(response, 404)
+
+    def test_image_attachment_traversal_without_permission(self):
+        # A traversal path that normalizes to a protected directory must still be denied.
+        traversal_path = 'foo/../' + self.image_attachment.image.name
+        url = reverse('media', kwargs={'path': traversal_path})
+        response = self.client.get(url)
+        self.assertHttpStatus(response, 404)
+
+    def test_device_type_with_permission(self):
+        self.add_permissions('dcim.view_devicetype')
+        url = reverse('media', kwargs={'path': self.device_type.front_image.name})
+        with patch('netbox.views.misc.serve', return_value=HttpResponse(status=200)):
+            response = self.client.get(url)
+        self.assertHttpStatus(response, 200)
+        self.assertEqual(response['Content-Disposition'], 'attachment')
+        self.assertEqual(response['X-Content-Type-Options'], 'nosniff')
+
+    def test_device_type_without_permission(self):
+        url = reverse('media', kwargs={'path': self.device_type.front_image.name})
+        response = self.client.get(url)
+        self.assertHttpStatus(response, 404)
+
+    def test_device_type_traversal_without_permission(self):
+        # A traversal path that normalizes to a protected directory must still be denied.
+        traversal_path = 'foo/../' + self.device_type.front_image.name
+        url = reverse('media', kwargs={'path': traversal_path})
+        response = self.client.get(url)
+        self.assertHttpStatus(response, 404)

netbox/netbox/views/misc.py

@@ -1,4 +1,5 @@
 import logging
+import posixpath
 import re
 from collections import namedtuple
 
@@ -6,6 +7,8 @@ from django.conf import settings
 from django.contrib import messages
 from django.contrib.contenttypes.models import ContentType
 from django.core.cache import cache
+from django.db.models import Q
+from django.http import Http404
 from django.shortcuts import redirect, render
 from django.utils.translation import gettext_lazy as _
 from django.views.generic import View
@@ -13,8 +16,10 @@ from django.views.static import serve
 from django_tables2 import RequestConfig
 from packaging import version
 
+from dcim.models import DeviceType
 from extras.constants import DEFAULT_DASHBOARD
 from extras.dashboard.utils import get_dashboard, get_default_dashboard
+from extras.models import ImageAttachment
 from netbox.forms import SearchForm
 from netbox.search import LookupTypes
 from netbox.search.backends import search_backend
@@ -131,7 +136,29 @@ class SearchView(ConditionalLoginRequiredMixin, View):
 
 class MediaView(TokenConditionalLoginRequiredMixin, View):
     """
-    Wrap Django's serve() view to enforce LOGIN_REQUIRED for static media.
+    Serve uploaded media files, enforcing authentication and view permission on the associated object.
     """
     def get(self, request, path):
-        return serve(request, path, document_root=settings.MEDIA_ROOT)
+
+        # Normalize the path to prevent traversal sequences (e.g. "foo/../image-attachments/...")
+        # from bypassing the directory checks below.
+        path = posixpath.normpath(path).lstrip('/')
+
+        # For known upload directories, resolve the path to an owning record and
+        # enforce object-level view permission. restrict() returns .none() when the
+        # user lacks permission, so a denial and a missing file are both 404s.
+        # Paths outside these directories (e.g. plugin uploads) fall through
+        # to the original behaviour.
+        if path.startswith('image-attachments/'):
+            if not ImageAttachment.objects.restrict(request.user, 'view').filter(image=path).exists():
+                raise Http404
+        elif path.startswith('devicetype-images/'):
+            if not DeviceType.objects.restrict(request.user, 'view').filter(
+                Q(front_image=path) | Q(rear_image=path)
+            ).exists():
+                raise Http404
+
+        response = serve(request, path, document_root=settings.MEDIA_ROOT)
+        response['Content-Disposition'] = 'attachment'
+        response['X-Content-Type-Options'] = 'nosniff'
+        return response