Skip to content

Commit c0e6168

Browse files
authored
Merge pull request #19347 from netbox-community/19346-redirect-checks
Fixes #19346: Ensure all redirect URLs are validated
2 parents 01da618 + e44ad8a commit c0e6168

File tree

6 files changed

+28
-9
lines changed

6 files changed

+28
-9
lines changed

netbox/account/views.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from django.shortcuts import render, resolve_url
1313
from django.urls import reverse
1414
from django.utils.decorators import method_decorator
15-
from django.utils.http import url_has_allowed_host_and_scheme, urlencode
15+
from django.utils.http import urlencode
1616
from django.utils.translation import gettext_lazy as _
1717
from django.views.decorators.debug import sensitive_post_parameters
1818
from django.views.generic import View
@@ -28,6 +28,7 @@
2828
from netbox.views import generic
2929
from users import forms, tables
3030
from users.models import UserConfig
31+
from utilities.request import safe_for_redirect
3132
from utilities.string import remove_linebreaks
3233
from utilities.views import register_model_view
3334

@@ -146,7 +147,7 @@ def redirect_to_next(self, request, logger):
146147
data = request.POST if request.method == "POST" else request.GET
147148
redirect_url = data.get('next', settings.LOGIN_REDIRECT_URL)
148149

149-
if redirect_url and url_has_allowed_host_and_scheme(redirect_url, allowed_hosts=None):
150+
if redirect_url and safe_for_redirect(redirect_url):
150151
logger.debug(f"Redirecting user to {remove_linebreaks(redirect_url)}")
151152
else:
152153
if redirect_url:

netbox/dcim/views.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from utilities.permissions import get_permission_for_model
2424
from utilities.query import count_related
2525
from utilities.query_functions import CollateAsChar
26+
from utilities.request import safe_for_redirect
2627
from utilities.views import (
2728
GetRelatedModelsMixin, GetReturnURLMixin, ObjectPermissionRequiredMixin, ViewTab, register_model_view
2829
)
@@ -3793,7 +3794,7 @@ def post(self, request, pk):
37933794
)
37943795
))
37953796

3796-
if '_addanother' in request.POST:
3797+
if '_addanother' in request.POST and safe_for_redirect(request.get_full_path()):
37973798
return redirect(request.get_full_path())
37983799

37993800
return redirect(self.get_return_url(request, device))

netbox/netbox/views/generic/bulk_views.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from utilities.htmx import htmx_partial
3030
from utilities.permissions import get_permission_for_model
3131
from utilities.query import reapply_model_ordering
32+
from utilities.request import safe_for_redirect
3233
from utilities.views import GetReturnURLMixin, get_viewname
3334
from .base import BaseMultiObjectView
3435
from .mixins import ActionsMixin, TableMixin
@@ -120,7 +121,10 @@ def export_template(self, template, request):
120121
# Strip the `export` param and redirect user to the filtered objects list
121122
query_params = request.GET.copy()
122123
query_params.pop('export')
123-
return redirect(f'{request.path}?{query_params.urlencode()}')
124+
redirect_url = f'{request.path}?{query_params.urlencode()}'
125+
if safe_for_redirect(redirect_url):
126+
return redirect(redirect_url)
127+
return redirect(get_viewname(self.queryset.model, 'list'))
124128

125129
#
126130
# Request handlers
@@ -284,7 +288,7 @@ def post(self, request):
284288
logger.info(msg)
285289
messages.success(request, msg)
286290

287-
if '_addanother' in request.POST:
291+
if '_addanother' in request.POST and safe_for_redirect(request.path):
288292
return redirect(request.path)
289293
return redirect(self.get_return_url(request))
290294

netbox/netbox/views/generic/object_views.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from utilities.htmx import htmx_partial
2121
from utilities.permissions import get_permission_for_model
2222
from utilities.querydict import normalize_querydict, prepare_cloned_fields
23+
from utilities.request import safe_for_redirect
2324
from utilities.views import GetReturnURLMixin, get_viewname
2425
from .base import BaseObjectView
2526
from .mixins import ActionsMixin, TableMixin
@@ -315,6 +316,8 @@ def post(self, request, *args, **kwargs):
315316
if 'return_url' in request.GET:
316317
params['return_url'] = request.GET.get('return_url')
317318
redirect_url += f"?{params.urlencode()}"
319+
if not safe_for_redirect(redirect_url):
320+
redirect_url = reverse('home')
318321

319322
return redirect(redirect_url)
320323

@@ -581,7 +584,7 @@ def post(self, request):
581584
))
582585

583586
# Redirect user on success
584-
if '_addanother' in request.POST:
587+
if '_addanother' in request.POST and safe_for_redirect(request.get_full_path()):
585588
return redirect(request.get_full_path())
586589
else:
587590
return redirect(self.get_return_url(request))

netbox/utilities/request.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
1+
from urllib.parse import urlparse
2+
3+
from django.utils.http import url_has_allowed_host_and_scheme
14
from django.utils.translation import gettext_lazy as _
25
from netaddr import AddrFormatError, IPAddress
3-
from urllib.parse import urlparse
46

57
from .constants import HTTP_REQUEST_META_SAFE_COPY
68

79
__all__ = (
810
'NetBoxFakeRequest',
911
'copy_safe_request',
1012
'get_client_ip',
13+
'safe_for_redirect',
1114
)
1215

1316

@@ -77,3 +80,10 @@ def get_client_ip(request, additional_headers=()):
7780

7881
# Could not determine the client IP address from request headers
7982
return None
83+
84+
85+
def safe_for_redirect(url):
86+
"""
87+
Returns True if the given URL is safe to use as an HTTP redirect; otherwise returns False.
88+
"""
89+
return url_has_allowed_host_and_scheme(url, allowed_hosts=None)

netbox/utilities/views.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@
55
from django.core.exceptions import ImproperlyConfigured
66
from django.urls import reverse
77
from django.urls.exceptions import NoReverseMatch
8-
from django.utils.http import url_has_allowed_host_and_scheme
98
from django.utils.translation import gettext_lazy as _
109

1110
from netbox.plugins import PluginConfig
1211
from netbox.registry import registry
1312
from utilities.relations import get_related_models
13+
from utilities.request import safe_for_redirect
1414
from .permissions import resolve_permission
1515

1616
__all__ = (
@@ -136,7 +136,7 @@ def get_return_url(self, request, obj=None):
136136
# First, see if `return_url` was specified as a query parameter or form data. Use this URL only if it's
137137
# considered safe.
138138
return_url = request.GET.get('return_url') or request.POST.get('return_url')
139-
if return_url and url_has_allowed_host_and_scheme(return_url, allowed_hosts=None):
139+
if return_url and safe_for_redirect(return_url):
140140
return return_url
141141

142142
# Next, check if the object being modified (if any) has an absolute URL.

0 commit comments

Comments
 (0)