From e44ad8af4512400c244589bbc34c1dc294792fc5 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 28 Apr 2025 14:27:49 -0400 Subject: [PATCH] Fixes #19346: Ensure all redirect URLs are validated --- netbox/account/views.py | 5 +++-- netbox/dcim/views.py | 3 ++- netbox/netbox/views/generic/bulk_views.py | 8 ++++++-- netbox/netbox/views/generic/object_views.py | 5 ++++- netbox/utilities/request.py | 12 +++++++++++- netbox/utilities/views.py | 4 ++-- 6 files changed, 28 insertions(+), 9 deletions(-) diff --git a/netbox/account/views.py b/netbox/account/views.py index 835ae81c2..ced2d595c 100644 --- a/netbox/account/views.py +++ b/netbox/account/views.py @@ -12,7 +12,7 @@ from django.shortcuts import get_object_or_404, redirect from django.shortcuts import render, resolve_url from django.urls import reverse from django.utils.decorators import method_decorator -from django.utils.http import url_has_allowed_host_and_scheme, urlencode +from django.utils.http import urlencode from django.utils.translation import gettext_lazy as _ from django.views.decorators.debug import sensitive_post_parameters from django.views.generic import View @@ -28,6 +28,7 @@ from netbox.config import get_config from netbox.views import generic from users import forms, tables from users.models import UserConfig +from utilities.request import safe_for_redirect from utilities.string import remove_linebreaks from utilities.views import register_model_view @@ -146,7 +147,7 @@ class LoginView(View): data = request.POST if request.method == "POST" else request.GET redirect_url = data.get('next', settings.LOGIN_REDIRECT_URL) - if redirect_url and url_has_allowed_host_and_scheme(redirect_url, allowed_hosts=None): + if redirect_url and safe_for_redirect(redirect_url): logger.debug(f"Redirecting user to {remove_linebreaks(redirect_url)}") else: if redirect_url: diff --git a/netbox/dcim/views.py b/netbox/dcim/views.py index 4eb3cf016..e878c9a86 100644 --- a/netbox/dcim/views.py +++ b/netbox/dcim/views.py @@ -23,6 +23,7 @@ from utilities.paginator import EnhancedPaginator, get_paginate_count from utilities.permissions import get_permission_for_model from utilities.query import count_related from utilities.query_functions import CollateAsChar +from utilities.request import safe_for_redirect from utilities.views import ( GetRelatedModelsMixin, GetReturnURLMixin, ObjectPermissionRequiredMixin, ViewTab, register_model_view ) @@ -3793,7 +3794,7 @@ class VirtualChassisAddMemberView(ObjectPermissionRequiredMixin, GetReturnURLMix ) )) - if '_addanother' in request.POST: + if '_addanother' in request.POST and safe_for_redirect(request.get_full_path()): return redirect(request.get_full_path()) return redirect(self.get_return_url(request, device)) diff --git a/netbox/netbox/views/generic/bulk_views.py b/netbox/netbox/views/generic/bulk_views.py index cf34e2293..373864695 100644 --- a/netbox/netbox/views/generic/bulk_views.py +++ b/netbox/netbox/views/generic/bulk_views.py @@ -29,6 +29,7 @@ from utilities.forms.bulk_import import BulkImportForm from utilities.htmx import htmx_partial from utilities.permissions import get_permission_for_model from utilities.query import reapply_model_ordering +from utilities.request import safe_for_redirect from utilities.views import GetReturnURLMixin, get_viewname from .base import BaseMultiObjectView from .mixins import ActionsMixin, TableMixin @@ -120,7 +121,10 @@ class ObjectListView(BaseMultiObjectView, ActionsMixin, TableMixin): # Strip the `export` param and redirect user to the filtered objects list query_params = request.GET.copy() query_params.pop('export') - return redirect(f'{request.path}?{query_params.urlencode()}') + redirect_url = f'{request.path}?{query_params.urlencode()}' + if safe_for_redirect(redirect_url): + return redirect(redirect_url) + return redirect(get_viewname(self.queryset.model, 'list')) # # Request handlers @@ -284,7 +288,7 @@ class BulkCreateView(GetReturnURLMixin, BaseMultiObjectView): logger.info(msg) messages.success(request, msg) - if '_addanother' in request.POST: + if '_addanother' in request.POST and safe_for_redirect(request.path): return redirect(request.path) return redirect(self.get_return_url(request)) diff --git a/netbox/netbox/views/generic/object_views.py b/netbox/netbox/views/generic/object_views.py index fb554ca4f..36d711439 100644 --- a/netbox/netbox/views/generic/object_views.py +++ b/netbox/netbox/views/generic/object_views.py @@ -20,6 +20,7 @@ from utilities.forms import ConfirmationForm, restrict_form_fields from utilities.htmx import htmx_partial from utilities.permissions import get_permission_for_model from utilities.querydict import normalize_querydict, prepare_cloned_fields +from utilities.request import safe_for_redirect from utilities.views import GetReturnURLMixin, get_viewname from .base import BaseObjectView from .mixins import ActionsMixin, TableMixin @@ -315,6 +316,8 @@ class ObjectEditView(GetReturnURLMixin, BaseObjectView): if 'return_url' in request.GET: params['return_url'] = request.GET.get('return_url') redirect_url += f"?{params.urlencode()}" + if not safe_for_redirect(redirect_url): + redirect_url = reverse('home') return redirect(redirect_url) @@ -581,7 +584,7 @@ class ComponentCreateView(GetReturnURLMixin, BaseObjectView): )) # Redirect user on success - if '_addanother' in request.POST: + if '_addanother' in request.POST and safe_for_redirect(request.get_full_path()): return redirect(request.get_full_path()) else: return redirect(self.get_return_url(request)) diff --git a/netbox/utilities/request.py b/netbox/utilities/request.py index 5571b326d..8af55e868 100644 --- a/netbox/utilities/request.py +++ b/netbox/utilities/request.py @@ -1,6 +1,8 @@ +from urllib.parse import urlparse + +from django.utils.http import url_has_allowed_host_and_scheme from django.utils.translation import gettext_lazy as _ from netaddr import AddrFormatError, IPAddress -from urllib.parse import urlparse from .constants import HTTP_REQUEST_META_SAFE_COPY @@ -8,6 +10,7 @@ __all__ = ( 'NetBoxFakeRequest', 'copy_safe_request', 'get_client_ip', + 'safe_for_redirect', ) @@ -77,3 +80,10 @@ def get_client_ip(request, additional_headers=()): # Could not determine the client IP address from request headers return None + + +def safe_for_redirect(url): + """ + Returns True if the given URL is safe to use as an HTTP redirect; otherwise returns False. + """ + return url_has_allowed_host_and_scheme(url, allowed_hosts=None) diff --git a/netbox/utilities/views.py b/netbox/utilities/views.py index 353dd927a..b8f65baf1 100644 --- a/netbox/utilities/views.py +++ b/netbox/utilities/views.py @@ -5,12 +5,12 @@ from django.contrib.auth.mixins import AccessMixin from django.core.exceptions import ImproperlyConfigured from django.urls import reverse from django.urls.exceptions import NoReverseMatch -from django.utils.http import url_has_allowed_host_and_scheme from django.utils.translation import gettext_lazy as _ from netbox.plugins import PluginConfig from netbox.registry import registry from utilities.relations import get_related_models +from utilities.request import safe_for_redirect from .permissions import resolve_permission __all__ = ( @@ -136,7 +136,7 @@ class GetReturnURLMixin: # First, see if `return_url` was specified as a query parameter or form data. Use this URL only if it's # considered safe. return_url = request.GET.get('return_url') or request.POST.get('return_url') - if return_url and url_has_allowed_host_and_scheme(return_url, allowed_hosts=None): + if return_url and safe_for_redirect(return_url): return return_url # Next, check if the object being modified (if any) has an absolute URL.