Fixes #19346: Ensure all redirect URLs are validated

This commit is contained in:
Jeremy Stretch 2025-04-28 14:27:49 -04:00
parent 81dfaf0d67
commit e44ad8af45
6 changed files with 28 additions and 9 deletions

View File

@ -12,7 +12,7 @@ from django.shortcuts import get_object_or_404, redirect
from django.shortcuts import render, resolve_url from django.shortcuts import render, resolve_url
from django.urls import reverse from django.urls import reverse
from django.utils.decorators import method_decorator 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.utils.translation import gettext_lazy as _
from django.views.decorators.debug import sensitive_post_parameters from django.views.decorators.debug import sensitive_post_parameters
from django.views.generic import View from django.views.generic import View
@ -28,6 +28,7 @@ from netbox.config import get_config
from netbox.views import generic from netbox.views import generic
from users import forms, tables from users import forms, tables
from users.models import UserConfig from users.models import UserConfig
from utilities.request import safe_for_redirect
from utilities.string import remove_linebreaks from utilities.string import remove_linebreaks
from utilities.views import register_model_view from utilities.views import register_model_view
@ -146,7 +147,7 @@ class LoginView(View):
data = request.POST if request.method == "POST" else request.GET data = request.POST if request.method == "POST" else request.GET
redirect_url = data.get('next', settings.LOGIN_REDIRECT_URL) 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)}") logger.debug(f"Redirecting user to {remove_linebreaks(redirect_url)}")
else: else:
if redirect_url: if redirect_url:

View File

@ -23,6 +23,7 @@ from utilities.paginator import EnhancedPaginator, get_paginate_count
from utilities.permissions import get_permission_for_model from utilities.permissions import get_permission_for_model
from utilities.query import count_related from utilities.query import count_related
from utilities.query_functions import CollateAsChar from utilities.query_functions import CollateAsChar
from utilities.request import safe_for_redirect
from utilities.views import ( from utilities.views import (
GetRelatedModelsMixin, GetReturnURLMixin, ObjectPermissionRequiredMixin, ViewTab, register_model_view 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(request.get_full_path())
return redirect(self.get_return_url(request, device)) return redirect(self.get_return_url(request, device))

View File

@ -29,6 +29,7 @@ from utilities.forms.bulk_import import BulkImportForm
from utilities.htmx import htmx_partial from utilities.htmx import htmx_partial
from utilities.permissions import get_permission_for_model from utilities.permissions import get_permission_for_model
from utilities.query import reapply_model_ordering from utilities.query import reapply_model_ordering
from utilities.request import safe_for_redirect
from utilities.views import GetReturnURLMixin, get_viewname from utilities.views import GetReturnURLMixin, get_viewname
from .base import BaseMultiObjectView from .base import BaseMultiObjectView
from .mixins import ActionsMixin, TableMixin 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 # Strip the `export` param and redirect user to the filtered objects list
query_params = request.GET.copy() query_params = request.GET.copy()
query_params.pop('export') 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 # Request handlers
@ -284,7 +288,7 @@ class BulkCreateView(GetReturnURLMixin, BaseMultiObjectView):
logger.info(msg) logger.info(msg)
messages.success(request, 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(request.path)
return redirect(self.get_return_url(request)) return redirect(self.get_return_url(request))

View File

@ -20,6 +20,7 @@ from utilities.forms import ConfirmationForm, restrict_form_fields
from utilities.htmx import htmx_partial from utilities.htmx import htmx_partial
from utilities.permissions import get_permission_for_model from utilities.permissions import get_permission_for_model
from utilities.querydict import normalize_querydict, prepare_cloned_fields from utilities.querydict import normalize_querydict, prepare_cloned_fields
from utilities.request import safe_for_redirect
from utilities.views import GetReturnURLMixin, get_viewname from utilities.views import GetReturnURLMixin, get_viewname
from .base import BaseObjectView from .base import BaseObjectView
from .mixins import ActionsMixin, TableMixin from .mixins import ActionsMixin, TableMixin
@ -315,6 +316,8 @@ class ObjectEditView(GetReturnURLMixin, BaseObjectView):
if 'return_url' in request.GET: if 'return_url' in request.GET:
params['return_url'] = request.GET.get('return_url') params['return_url'] = request.GET.get('return_url')
redirect_url += f"?{params.urlencode()}" redirect_url += f"?{params.urlencode()}"
if not safe_for_redirect(redirect_url):
redirect_url = reverse('home')
return redirect(redirect_url) return redirect(redirect_url)
@ -581,7 +584,7 @@ class ComponentCreateView(GetReturnURLMixin, BaseObjectView):
)) ))
# Redirect user on success # 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()) return redirect(request.get_full_path())
else: else:
return redirect(self.get_return_url(request)) return redirect(self.get_return_url(request))

View File

@ -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 django.utils.translation import gettext_lazy as _
from netaddr import AddrFormatError, IPAddress from netaddr import AddrFormatError, IPAddress
from urllib.parse import urlparse
from .constants import HTTP_REQUEST_META_SAFE_COPY from .constants import HTTP_REQUEST_META_SAFE_COPY
@ -8,6 +10,7 @@ __all__ = (
'NetBoxFakeRequest', 'NetBoxFakeRequest',
'copy_safe_request', 'copy_safe_request',
'get_client_ip', '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 # Could not determine the client IP address from request headers
return None 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)

View File

@ -5,12 +5,12 @@ from django.contrib.auth.mixins import AccessMixin
from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ImproperlyConfigured
from django.urls import reverse from django.urls import reverse
from django.urls.exceptions import NoReverseMatch 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 django.utils.translation import gettext_lazy as _
from netbox.plugins import PluginConfig from netbox.plugins import PluginConfig
from netbox.registry import registry from netbox.registry import registry
from utilities.relations import get_related_models from utilities.relations import get_related_models
from utilities.request import safe_for_redirect
from .permissions import resolve_permission from .permissions import resolve_permission
__all__ = ( __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 # First, see if `return_url` was specified as a query parameter or form data. Use this URL only if it's
# considered safe. # considered safe.
return_url = request.GET.get('return_url') or request.POST.get('return_url') 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 return return_url
# Next, check if the object being modified (if any) has an absolute URL. # Next, check if the object being modified (if any) has an absolute URL.