Browse Source

#22640 - Enforce ALLOWED_URL_SCHEMES for URLs in custom fields (#22732)

Arthur Hanson 4 weeks ago
parent
commit
58b4209fe6

+ 1 - 1
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).
 
 ---
 

+ 1 - 1
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

+ 11 - 0
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}

+ 7 - 1
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))
 

+ 32 - 0
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})

+ 7 - 1
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('<i class="mdi mdi-close-thick text-danger"></i>')
         if self.customfield.type == CustomFieldTypeChoices.TYPE_URL:
-            return mark_safe(f'<a href="{escape(value)}">{escape(value)}</a>')
+            # 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'<a href="{escape(value)}">{escape(value)}</a>')
+            return escape(value)
         if self.customfield.type == CustomFieldTypeChoices.TYPE_SELECT:
             return self.customfield.get_choice_label(value)
         if self.customfield.type == CustomFieldTypeChoices.TYPE_MULTISELECT:

+ 28 - 0
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'), '<a href="https://example.com">https://example.com</a>')
+
+    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, '<a href="javascript%3Aalert(1)">javascript%3Aalert(1)</a>')

+ 5 - 1
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 %}
-  <a href="{{ value }}">{{ value|truncatechars:70 }}</a>
+  {% if url_allowed %}
+    <a href="{{ value }}">{{ value|truncatechars:70 }}</a>
+  {% else %}
+    {{ value|truncatechars:70 }}
+  {% endif %}
 {% elif customfield.type == 'json' and value is not None %}
   <pre>{{ value|json }}</pre>
 {% elif customfield.type == 'select' and value %}

+ 9 - 0
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,
     }
 
 

+ 26 - 0
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('<a href="https://example.com">https://example.com</a>', 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('<a href="javascript%3Aalert(1)">javascript%3Aalert(1)</a>', html)
+
 
 class StaticWithParamsTestCase(TestCase):
     """

+ 19 - 0
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