From 58b4209fe6522c7bcd6a57cdb1a510f172b118c6 Mon Sep 17 00:00:00 2001 From: Arthur Hanson Date: Thu, 23 Jul 2026 07:33:53 -0700 Subject: [PATCH] #22640 - Enforce ALLOWED_URL_SCHEMES for URLs in custom fields (#22732) --- docs/configuration/security.md | 2 +- docs/customization/custom-fields.md | 2 +- netbox/extras/api/customfields.py | 11 +++++++ netbox/extras/models/customfields.py | 8 ++++- netbox/extras/tests/test_customfields.py | 32 +++++++++++++++++++ netbox/netbox/tables/columns.py | 8 ++++- netbox/netbox/tests/test_tables.py | 28 ++++++++++++++++ .../templates/builtins/customfield_value.html | 6 +++- .../utilities/templatetags/builtins/tags.py | 9 ++++++ netbox/utilities/tests/test_templatetags.py | 26 +++++++++++++++ netbox/utilities/validators.py | 19 +++++++++++ 11 files changed, 146 insertions(+), 5 deletions(-) diff --git a/docs/configuration/security.md b/docs/configuration/security.md index 38dcf5d51..044e6c4a5 100644 --- a/docs/configuration/security.md +++ b/docs/configuration/security.md @@ -6,7 +6,7 @@ Default: `('file', 'ftp', 'ftps', 'http', 'https', 'irc', 'mailto', 'sftp', 'ssh', 'tel', 'telnet', 'tftp', 'vnc', 'xmpp')` -A list of permitted URL schemes referenced when rendering links within NetBox. Note that only the schemes specified in this list will be accepted: If adding your own, be sure to replicate all the default values as well (excluding those schemes which are not desirable). +A list of permitted URL schemes referenced when rendering links within NetBox. This list is also enforced when validating the value of URL custom fields. Note that only the schemes specified in this list will be accepted: If adding your own, be sure to replicate all the default values as well (excluding those schemes which are not desirable). --- diff --git a/docs/customization/custom-fields.md b/docs/customization/custom-fields.md index 0f569b2a1..51aa77183 100644 --- a/docs/customization/custom-fields.md +++ b/docs/customization/custom-fields.md @@ -17,7 +17,7 @@ Custom fields may be created by navigating to Customization > Custom Fields. Net * Boolean: True or false * Date: A date in ISO 8601 format (YYYY-MM-DD) * Date & time: A date and time in ISO 8601 format (YYYY-MM-DD HH:MM:SS) -* URL: This will be presented as a link in the web UI +* URL: This will be presented as a link in the web UI. Values are restricted to the schemes permitted by [`ALLOWED_URL_SCHEMES`](../configuration/security.md#allowed_url_schemes). A value entered without a scheme (e.g. `example.com`) is assumed to use `https` and stored as an absolute URL (e.g. `https://example.com`). * JSON: Arbitrary data stored in JSON format * Selection: A selection of one of several pre-defined custom choices * Multiple selection: A selection field which supports the assignment of multiple values diff --git a/netbox/extras/api/customfields.py b/netbox/extras/api/customfields.py index 2c36870e4..953bedafa 100644 --- a/netbox/extras/api/customfields.py +++ b/netbox/extras/api/customfields.py @@ -1,3 +1,4 @@ +from django.core.exceptions import ValidationError as DjangoValidationError from django.utils.translation import gettext as _ from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import extend_schema_field @@ -8,6 +9,7 @@ from extras.choices import CustomFieldTypeChoices from extras.constants import CUSTOMFIELD_EMPTY_VALUES from extras.models import CustomField from utilities.api import get_serializer_for_model +from utilities.forms.fields import LaxURLField # # Custom fields @@ -120,6 +122,15 @@ class CustomFieldsDataField(Field): else: raise ValidationError(_("Unknown related object(s): {name}").format(name=data[cf.name])) + # Normalize URL values the same way the UI does (LaxURLField with assume_scheme='https'), so a + # schemeless value (e.g. "example.com") is stored as an absolute URL ("https://example.com"). + # Malformed values are left untouched for CustomField.validate() to report. + elif cf.type == CustomFieldTypeChoices.TYPE_URL and isinstance(data.get(cf.name), str) and data[cf.name]: + try: + data[cf.name] = LaxURLField(assume_scheme='https').to_python(data[cf.name]) + except DjangoValidationError: + pass + # If updating an existing instance, start with existing custom_field_data if self.parent.instance: data = {**self.parent.instance.custom_field_data, **data} diff --git a/netbox/extras/models/customfields.py b/netbox/extras/models/customfields.py index ae5c73dc9..c2f0250b9 100644 --- a/netbox/extras/models/customfields.py +++ b/netbox/extras/models/customfields.py @@ -45,7 +45,7 @@ from utilities.forms.widgets import APISelect, APISelectMultiple, DatePicker, Da from utilities.jsonschema import validate_schema from utilities.querysets import RestrictedQuerySet, chunked_update from utilities.templatetags.builtins.filters import render_markdown -from utilities.validators import validate_regex +from utilities.validators import url_scheme_is_allowed, validate_regex __all__ = ( 'CustomField', @@ -769,6 +769,12 @@ class CustomField(CloningMixin, ExportTemplatesMixin, OwnerMixin, ChangeLoggedMo elif self.type == CustomFieldTypeChoices.TYPE_URL: if type(value) is not str: raise ValidationError(_("Value must be a string.")) + # Enforce ALLOWED_URL_SCHEMES to guard against dangerous schemes (e.g. javascript:). A + # schemeless value is permitted and treated as relative. + if not url_scheme_is_allowed(value): + raise ValidationError( + _("URLs must use a scheme permitted by ALLOWED_URL_SCHEMES.") + ) if self.validation_regex and not re.match(self.validation_regex, value): raise ValidationError(_("Value must match regex '{regex}'").format(regex=self.validation_regex)) diff --git a/netbox/extras/tests/test_customfields.py b/netbox/extras/tests/test_customfields.py index 8f402e787..799fbe287 100644 --- a/netbox/extras/tests/test_customfields.py +++ b/netbox/extras/tests/test_customfields.py @@ -1692,6 +1692,38 @@ class CustomFieldAPITestCase(APITestCase): response = self.client.patch(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_200_OK) + def test_url_scheme_validation(self): + """ + Test that URL custom field values must use a scheme permitted by ALLOWED_URL_SCHEMES (fixes + #22640), and that a schemeless value is normalized to an absolute URL (assume_scheme='https'), + consistent with the UI. + """ + site2 = Site.objects.get(name='Site 2') + url = reverse('dcim-api:site-detail', kwargs={'pk': site2.pk}) + self.add_permissions('dcim.change_site') + + # A dangerous scheme (e.g. javascript:) must be rejected + data = {'custom_fields': {'url_field': 'javascript:alert(1)'}} + response = self.client.patch(url, data, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) + + # A well-formed URL using a scheme outside ALLOWED_URL_SCHEMES must be rejected + data = {'custom_fields': {'url_field': 'gopher://example.com'}} + response = self.client.patch(url, data, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) + + # An allowed scheme must be accepted + data = {'custom_fields': {'url_field': 'https://example.com'}} + response = self.client.patch(url, data, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) + + # A schemeless value must be accepted and normalized to https, matching the UI + data = {'custom_fields': {'url_field': 'example.com'}} + response = self.client.patch(url, data, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) + site2.refresh_from_db() + self.assertEqual(site2.custom_field_data['url_field'], 'https://example.com') + def test_json_schema_validation(self): site2 = Site.objects.get(name='Site 2') url = reverse('dcim-api:site-detail', kwargs={'pk': site2.pk}) diff --git a/netbox/netbox/tables/columns.py b/netbox/netbox/tables/columns.py index cd6ae2421..3632ffd76 100644 --- a/netbox/netbox/tables/columns.py +++ b/netbox/netbox/tables/columns.py @@ -23,6 +23,7 @@ from utilities.object_types import object_type_identifier, object_type_name from utilities.permissions import get_permission_for_model from utilities.request import get_safe_request_context from utilities.templatetags.builtins.filters import render_markdown +from utilities.validators import url_scheme_is_allowed from utilities.views import get_action_url __all__ = ( @@ -562,7 +563,12 @@ class CustomFieldColumn(tables.Column): if self.customfield.type == CustomFieldTypeChoices.TYPE_BOOLEAN and value is False: return mark_safe('') if self.customfield.type == CustomFieldTypeChoices.TYPE_URL: - return mark_safe(f'{escape(value)}') + # Only render as a link if the scheme is permitted by ALLOWED_URL_SCHEMES, to guard against + # dangerous schemes (e.g. javascript:) in values which bypassed validation. A schemeless + # (relative) value is considered safe. + if url_scheme_is_allowed(value): + return mark_safe(f'{escape(value)}') + return escape(value) if self.customfield.type == CustomFieldTypeChoices.TYPE_SELECT: return self.customfield.get_choice_label(value) if self.customfield.type == CustomFieldTypeChoices.TYPE_MULTISELECT: diff --git a/netbox/netbox/tests/test_tables.py b/netbox/netbox/tests/test_tables.py index aa6fd5ad2..c4cca0d4b 100644 --- a/netbox/netbox/tests/test_tables.py +++ b/netbox/netbox/tests/test_tables.py @@ -4,6 +4,8 @@ from django.test import RequestFactory, TestCase from dcim.models import Device, Site from dcim.tables import DeviceTable +from extras.choices import CustomFieldTypeChoices +from extras.models import CustomField from netbox.tables import NetBoxTable, columns from utilities.testing import create_tags, create_test_device, create_test_user @@ -119,3 +121,29 @@ class TagColumnTestCase(TestCase): 'table': table }) template.render(context) + + +class CustomFieldColumnTestCase(TestCase): + """ + A URL custom field value is rendered directly into an href, so its scheme must be validated + against ALLOWED_URL_SCHEMES to avoid rendering dangerous schemes (e.g. javascript:) as clickable + links (fixes #22640). + """ + + def _render(self, value): + customfield = CustomField(name='url_field', type=CustomFieldTypeChoices.TYPE_URL) + return columns.CustomFieldColumn(customfield).render(value) + + def test_url_allowed_scheme_rendered_as_link(self): + self.assertEqual(self._render('https://example.com'), 'https://example.com') + + def test_url_disallowed_scheme_not_rendered_as_link(self): + rendered = self._render('javascript:alert(1)') + self.assertNotIn('href', rendered) + self.assertIn('javascript:alert(1)', rendered) + + def test_url_percent_encoded_scheme_rendered_as_relative_link(self): + # A percent-encoded scheme is inert: a browser will not decode "%3A" to execute javascript:, + # so the value has no scheme and is rendered as a link as-is. + rendered = self._render('javascript%3Aalert(1)') + self.assertEqual(rendered, 'javascript%3Aalert(1)') diff --git a/netbox/utilities/templates/builtins/customfield_value.html b/netbox/utilities/templates/builtins/customfield_value.html index cb09ed262..68ddb2d4b 100644 --- a/netbox/utilities/templates/builtins/customfield_value.html +++ b/netbox/utilities/templates/builtins/customfield_value.html @@ -15,7 +15,11 @@ {% elif customfield.type == 'datetime' and value %} {{ value|isodatetime }} {% elif customfield.type == 'url' and value %} - {{ value|truncatechars:70 }} + {% if url_allowed %} + {{ value|truncatechars:70 }} + {% else %} + {{ value|truncatechars:70 }} + {% endif %} {% elif customfield.type == 'json' and value is not None %} 
{{ value|json }}
{% elif customfield.type == 'select' and value %} diff --git a/netbox/utilities/templatetags/builtins/tags.py b/netbox/utilities/templatetags/builtins/tags.py index 0f01f9570..cac250573 100644 --- a/netbox/utilities/templatetags/builtins/tags.py +++ b/netbox/utilities/templatetags/builtins/tags.py @@ -7,6 +7,7 @@ from django.utils.safestring import mark_safe from extras.choices import CustomFieldTypeChoices from utilities.querydict import dict_to_querydict +from utilities.validators import url_scheme_is_allowed __all__ = ( 'badge', @@ -48,6 +49,8 @@ def customfield_value(customfield, value): """ color = None value_has_colors = False + # Determines whether a URL value may be rendered as a clickable link + url_allowed = False if value: if customfield.type == CustomFieldTypeChoices.TYPE_SELECT: @@ -58,11 +61,17 @@ def customfield_value(customfield, value): value_has_colors = any(choice_color for _, choice_color in value) if not value_has_colors: value = [choice_label for choice_label, _ in value] + elif customfield.type == CustomFieldTypeChoices.TYPE_URL: + # Only render as a link if the scheme is permitted by ALLOWED_URL_SCHEMES. This guards against + # dangerous schemes (e.g. javascript:) in values stored before validation was enforced or via + # paths which bypass model validation. A schemeless (relative) value is considered safe. + url_allowed = url_scheme_is_allowed(value) return { 'customfield': customfield, 'value': value, 'color': color, 'value_has_colors': value_has_colors, + 'url_allowed': url_allowed, } diff --git a/netbox/utilities/tests/test_templatetags.py b/netbox/utilities/tests/test_templatetags.py index 7da38f163..81276558e 100644 --- a/netbox/utilities/tests/test_templatetags.py +++ b/netbox/utilities/tests/test_templatetags.py @@ -39,6 +39,15 @@ class CustomFieldValueTagTestCase(TestCase): ) cls.multiselect_field.object_types.set([object_type]) + cls.url_field = CustomField.objects.create( + name='url_field', + type=CustomFieldTypeChoices.TYPE_URL, + ) + cls.url_field.object_types.set([object_type]) + + def _render(self, customfield, value): + return render_to_string('builtins/customfield_value.html', customfield_value(customfield, value)) + def test_select_choice_context_includes_color(self): context = customfield_value(self.select_field, 'a') @@ -63,6 +72,23 @@ class CustomFieldValueTagTestCase(TestCase): self.assertFalse(context['value_has_colors']) self.assertEqual(context['value'], ['Option B']) + def test_url_allowed_scheme_rendered_as_link(self): + html = self._render(self.url_field, 'https://example.com') + self.assertInHTML('https://example.com', html) + + def test_url_disallowed_scheme_not_rendered_as_link(self): + # A dangerous scheme (e.g. one stored before validation was enforced) must not become a + # clickable href (fixes #22640). + html = self._render(self.url_field, 'javascript:alert(1)') + self.assertNotIn('href', html) + self.assertIn('javascript:alert(1)', html) + + def test_url_percent_encoded_scheme_rendered_as_relative_link(self): + # A percent-encoded scheme is inert: a browser will not decode "%3A" to execute javascript:, + # so the value has no scheme and is rendered as a link as-is. + html = self._render(self.url_field, 'javascript%3Aalert(1)') + self.assertInHTML('javascript%3Aalert(1)', html) + class StaticWithParamsTestCase(TestCase): """ diff --git a/netbox/utilities/validators.py b/netbox/utilities/validators.py index 47622d4d5..5fceb817c 100644 --- a/netbox/utilities/validators.py +++ b/netbox/utilities/validators.py @@ -1,5 +1,6 @@ import decimal import re +from urllib.parse import urlparse from django.core.exceptions import ValidationError from django.core.validators import BaseValidator, RegexValidator, URLValidator, _lazy_re_compile @@ -12,6 +13,7 @@ __all__ = ( 'EnhancedURLValidator', 'ExclusionValidator', 'MultipleOfValidator', + 'url_scheme_is_allowed', 'validate_regex', ) @@ -72,6 +74,23 @@ class MultipleOfValidator(BaseValidator): ) +def url_scheme_is_allowed(value): + """ + Return True if the URL's scheme is permitted by ALLOWED_URL_SCHEMES. A schemeless (relative) value + is considered permitted. + + The scheme is compared in lower case. A percent-encoded scheme (e.g. "javascript%3A…") yields no + scheme, matching browser behavior: a browser does not decode the scheme portion of an href, so such + a value is inert and is treated as relative. A malformed URL which cannot be parsed (e.g. + "http://[::1/foo") likewise yields no scheme. + """ + try: + scheme = urlparse(value).scheme.lower() + except ValueError: + scheme = '' + return not scheme or scheme in get_config().ALLOWED_URL_SCHEMES + + def validate_regex(value): """ Checks that the value is a valid regular expression. (Don't confuse this with RegexValidator, which *uses* a regex