Преглед изворни кода

Closes #22753: Add `header_safe` Jinja2 filter for sanitizing webhook headers (#22754)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Jeremy Stretch пре 5 дана
родитељ
комит
728c84470b

+ 10 - 0
docs/integrations/webhooks.md

@@ -41,6 +41,16 @@ The following data is available as context for Jinja2 templates:
 
     Use `request.user` and `request.id` from the `request` object included in the callback context instead.
 
+### Sanitizing Header Values
+
+When rendering the `additional_headers` field, a `header_safe` filter is made available for sanitizing a value for safe inclusion in a raw HTTP header. It strips newlines and other control characters from the rendered value, preventing HTTP header (CR/LF) injection.
+
+Whenever a header value incorporates data which may be influenced by other users (such as an object's attributes), pass it through this filter to avoid smuggling of additional headers. For example:
+
+```
+X-Object-Name: {{ data.name | header_safe }}
+```
+
 ### Default Request Body
 
 If no body template is specified, the request body will be populated with a JSON object containing the context data. For example, a newly created site might appear as follows:

+ 7 - 0
docs/models/extras/webhook.md

@@ -52,6 +52,13 @@ The content type to indicate in the outgoing HTTP request header. See [this list
 
 Any additional header to include with the outgoing HTTP request. These should be defined in the format `Name: Value`, with each header on a separate line. Jinja2 templating is supported for this field.
 
+!!! warning "Sanitize interpolated header values"
+    When interpolating data which may be influenced by other users (such as object attributes) into a header value, apply the `header_safe` filter to guard against HTTP header (CR/LF) injection. This filter strips newlines and other control characters which could otherwise be used to smuggle additional headers into the request. For example:
+
+    ```
+    X-Object-Name: {{ data.name | header_safe }}
+    ```
+
 ### Body Template
 
 Jinja2 template for a custom request body, if desired. If not defined, NetBox will populate the request body with a raw dump of the webhook context.

+ 9 - 3
netbox/extras/models/models.py

@@ -34,7 +34,7 @@ from netbox.models.features import (
 )
 from netbox.models.mixins import OwnerMixin
 from utilities.html import clean_html
-from utilities.jinja2 import render_jinja2
+from utilities.jinja2 import render_jinja2, sanitize_http_header
 from utilities.querydict import dict_to_querydict
 from utilities.querysets import RestrictedQuerySet
 from utilities.tables import get_table_for_model
@@ -212,7 +212,9 @@ class Webhook(CustomFieldsMixin, ExportTemplatesMixin, TagsMixin, OwnerMixin, Ch
         help_text=_(
             "User-supplied HTTP headers to be sent with the request in addition to the HTTP content type. Headers "
             "should be defined in the format <code>Name: Value</code>. Jinja2 template processing is supported with "
-            "the same context as the request body (below)."
+            "the same context as the request body (below). When interpolating untrusted data (such as object "
+            "attributes) into a header value, apply the <code>header_safe</code> filter to guard against HTTP header "
+            "injection, e.g. <code>X-Object: {{ data.name | header_safe }}</code>."
         )
     )
     body_template = models.TextField(
@@ -284,8 +286,12 @@ class Webhook(CustomFieldsMixin, ExportTemplatesMixin, TagsMixin, OwnerMixin, Ch
         if not self.additional_headers:
             return {}
         ret = {}
-        data = render_jinja2(self.additional_headers, context)
+        # Expose the `header_safe` filter so template authors can sanitize interpolated values (e.g. user-controlled
+        # object data) against HTTP header (CR/LF) injection. See utilities.jinja2.sanitize_http_header.
+        data = render_jinja2(self.additional_headers, context, filters={'header_safe': sanitize_http_header})
         for line in data.splitlines():
+            if ':' not in line:
+                continue
             header, value = line.split(':', 1)
             ret[header.strip()] = value.strip()
         return ret

+ 69 - 1
netbox/extras/tests/test_event_rules.py

@@ -8,7 +8,7 @@ from unittest.mock import Mock, patch
 import django_rq
 from django.conf import settings
 from django.http import HttpResponse
-from django.test import RequestFactory, tag
+from django.test import RequestFactory, TestCase, tag
 from django.urls import reverse
 from PIL import Image
 from requests import Session
@@ -801,3 +801,71 @@ class EventRuleTestCase(RQQueueTestMixin, APITestCase):
         job = self.queue.jobs[0]
         self.assertEqual(job.kwargs['event_rule'], event_rule)
         self.assertEqual(job.kwargs['event_type'], OBJECT_UPDATED)
+
+
+class WebhookRenderHeadersTest(TestCase):
+
+    def test_render_headers(self):
+        """Basic header rendering with Jinja2 interpolation."""
+        webhook = Webhook(
+            name='Webhook 1',
+            payload_url='http://localhost:9000/',
+            additional_headers='X-Foo: Bar\nX-Object: {{ data.name }}',
+        )
+        headers = webhook.render_headers({'data': {'name': 'Site 1'}})
+        self.assertEqual(headers, {'X-Foo': 'Bar', 'X-Object': 'Site 1'})
+
+    def test_render_headers_multiline_block(self):
+        """A multi-line Jinja2 block (e.g. a loop generating headers) must render against the full template."""
+        webhook = Webhook(
+            name='Webhook 1',
+            payload_url='http://localhost:9000/',
+            additional_headers=(
+                '{% for k, v in data.headers.items() %}X-{{ k }}: {{ v }}\n'
+                '{% endfor %}'
+            ),
+        )
+        headers = webhook.render_headers({'data': {'headers': {'Foo': '1', 'Bar': '2'}}})
+        self.assertEqual(headers, {'X-Foo': '1', 'X-Bar': '2'})
+
+    def test_render_headers_skips_blank_lines(self):
+        """Blank lines in the rendered output (e.g. from Jinja2 block tags) must be skipped, not raise."""
+        webhook = Webhook(
+            name='Webhook 1',
+            payload_url='http://localhost:9000/',
+            # Block tags on their own lines leave behind blank lines once rendered
+            additional_headers=(
+                '{% for k, v in data.headers.items() %}\n'
+                'X-{{ k }}: {{ v }}\n'
+                '{% endfor %}'
+            ),
+        )
+        headers = webhook.render_headers({'data': {'headers': {'Foo': '1', 'Bar': '2'}}})
+        self.assertEqual(headers, {'X-Foo': '1', 'X-Bar': '2'})
+
+    def test_render_headers_skips_lines_without_separator(self):
+        """A non-blank line lacking a 'Name: Value' separator must be skipped, not raise."""
+        webhook = Webhook(
+            name='Webhook 1',
+            payload_url='http://localhost:9000/',
+            additional_headers='X-Foo: Bar\nthis line has no colon\nX-Baz: Qux',
+        )
+        headers = webhook.render_headers({})
+        self.assertEqual(headers, {'X-Foo': 'Bar', 'X-Baz': 'Qux'})
+
+    def test_render_headers_header_safe_filter_available(self):
+        """
+        The `header_safe` filter must be available when rendering headers, and must strip control characters
+        (including CR/LF) so that untrusted data cannot smuggle additional headers via CR/LF injection.
+        """
+        webhook = Webhook(
+            name='Webhook 1',
+            payload_url='http://localhost:9000/',
+            additional_headers='X-Object: {{ data.name | header_safe }}',
+        )
+        headers = webhook.render_headers({'data': {'name': 'legit\r\nX-Injected: evil\x00'}})
+
+        # The injected newline is stripped, so only a single (sanitized) header is produced
+        self.assertEqual(list(headers.keys()), ['X-Object'])
+        self.assertNotIn('X-Injected', headers)
+        self.assertEqual(headers['X-Object'], 'legitX-Injected: evil')

+ 43 - 1
netbox/extras/tests/test_models.py

@@ -36,7 +36,7 @@ from extras.models import (
 from extras.models.mixins import RenderTemplateMixin
 from tenancy.models import Tenant, TenantGroup
 from utilities.exceptions import AbortRequest
-from utilities.jinja2 import env_filter, render_jinja2
+from utilities.jinja2 import env_filter, render_jinja2, sanitize_http_header
 from utilities.tables import get_table_for_model
 from virtualization.models import Cluster, ClusterGroup, ClusterType, VirtualMachine
 
@@ -1274,6 +1274,48 @@ class JinjaEnvFilterTestCase(TestCase):
             self.assertEqual(output, 'overridden')
 
 
+class SanitizeHTTPHeaderFilterTestCase(TestCase):
+    """
+    Tests for the sanitize_http_header() Jinja2 filter (exposed as `header_safe`) and the render_jinja2()
+    `filters` argument used to make it available.
+    """
+
+    def test_strips_crlf(self):
+        self.assertEqual(sanitize_http_header('legit\r\nX-Injected: evil'), 'legitX-Injected: evil')
+
+    def test_strips_control_characters(self):
+        self.assertEqual(sanitize_http_header('foo\x00\x1f\x7fbar'), 'foobar')
+
+    def test_preserves_normal_value(self):
+        self.assertEqual(sanitize_http_header('application/json'), 'application/json')
+
+    def test_coerces_non_string(self):
+        self.assertEqual(sanitize_http_header(42), '42')
+
+    def test_available_via_render_filters_argument(self):
+        output = render_jinja2(
+            "{{ value | header_safe }}",
+            {'value': 'a\r\nb'},
+            filters={'header_safe': sanitize_http_header},
+        )
+        self.assertEqual(output, 'ab')
+
+    def test_render_filters_take_precedence_over_user_config(self):
+        # A per-render filter cannot be shadowed by a user-configured filter of the same name
+        with self.settings(JINJA2_FILTERS={'header_safe': lambda v: 'shadowed'}):
+            output = render_jinja2(
+                "{{ value | header_safe }}",
+                {'value': 'a\r\nb'},
+                filters={'header_safe': sanitize_http_header},
+            )
+            self.assertEqual(output, 'ab')
+
+    def test_not_registered_without_filters_argument(self):
+        # The filter must not leak into general-purpose rendering
+        with self.assertRaises(TemplateError):
+            render_jinja2("{{ 'x' | header_safe }}", {})
+
+
 class ExportTemplateContextTestCase(TestCase):
     """
     Tests for ExportTemplate.get_context() including public model population.

+ 23 - 4
netbox/utilities/jinja2.py

@@ -1,5 +1,6 @@
 import fnmatch
 import os
+import re
 
 from django.apps import apps
 from jinja2 import BaseLoader, TemplateNotFound
@@ -13,8 +14,13 @@ __all__ = (
     'DataFileLoader',
     'env_filter',
     'render_jinja2',
+    'sanitize_http_header',
 )
 
+# Control characters (C0 range plus DEL) which are invalid in an HTTP header value. Notably, this includes the
+# carriage return and line feed characters used to smuggle additional headers (CR/LF injection).
+HTTP_HEADER_INVALID_CHARS_RE = re.compile(r'[\x00-\x1f\x7f]')
+
 
 def env_filter(name):
     """
@@ -28,6 +34,15 @@ def env_filter(name):
     return os.environ.get(name)
 
 
+def sanitize_http_header(value):
+    """
+    Jinja2 filter which sanitizes a value for safe inclusion in a raw HTTP header by stripping newlines and other
+    control characters. This guards against HTTP header (CR/LF) injection when interpolating untrusted data (e.g.
+    user-controlled object attributes) into a webhook's additional headers.
+    """
+    return HTTP_HEADER_INVALID_CHARS_RE.sub('', str(value))
+
+
 DEFAULT_JINJA2_FILTERS = {
     'env': env_filter,
 }
@@ -71,11 +86,14 @@ class DataFileLoader(BaseLoader):
 # Utility functions
 #
 
-def render_jinja2(template_code, context, environment_params=None, data_file=None, debug=False):
+def render_jinja2(template_code, context, environment_params=None, data_file=None, debug=False, filters=None):
     """
     Render a Jinja2 template with the provided context. Return the rendered content.
 
     If debug is True, the Jinja2 debug extension is enabled to assist with template development.
+
+    The optional `filters` argument is a mapping of additional Jinja2 filters to make available for this render only
+    (e.g. context-specific sanitization filters). These take precedence over the default and user-configured filters.
     """
     environment_params = dict(environment_params or {})
 
@@ -98,9 +116,10 @@ def render_jinja2(template_code, context, environment_params=None, data_file=Non
     environment = SandboxedEnvironment(**environment_params)
 
     # Register default filters, then apply any user-defined filters. User-defined entries take precedence so that
-    # existing JINJA2_FILTERS configurations are never overridden.
-    filters = {**DEFAULT_JINJA2_FILTERS, **get_config().JINJA2_FILTERS}
-    environment.filters.update(filters)
+    # existing JINJA2_FILTERS configurations are never overridden. Any filters passed for this render take precedence
+    # over both so that context-specific (e.g. sanitization) filters cannot be shadowed.
+    all_filters = {**DEFAULT_JINJA2_FILTERS, **get_config().JINJA2_FILTERS, **(filters or {})}
+    environment.filters.update(all_filters)
 
     if data_file:
         template = environment.get_template(data_file.path)