From 87c53aaaeb7f1cfe538c75241d4a5179a7ca81df Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 8 Jun 2026 10:05:39 -0400 Subject: [PATCH] Fixes #22399: Enforce object permissions for relevant static media (#22400) --- netbox/netbox/tests/test_views.py | 69 ++++++++++++++++++++++++++++++- netbox/netbox/views/misc.py | 31 +++++++++++++- 2 files changed, 97 insertions(+), 3 deletions(-) diff --git a/netbox/netbox/tests/test_views.py b/netbox/netbox/tests/test_views.py index ee9599dc2..96c31ba16 100644 --- a/netbox/netbox/tests/test_views.py +++ b/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) diff --git a/netbox/netbox/views/misc.py b/netbox/netbox/views/misc.py index 29833fba9..2eeba7cb8 100644 --- a/netbox/netbox/views/misc.py +++ b/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