diff --git a/docs/integrations/webhooks.md b/docs/integrations/webhooks.md index fe5d15b9b..7f9983da6 100644 --- a/docs/integrations/webhooks.md +++ b/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: diff --git a/docs/models/extras/webhook.md b/docs/models/extras/webhook.md index 7f0f322c6..1d17bdd49 100644 --- a/docs/models/extras/webhook.md +++ b/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. diff --git a/netbox/extras/models/models.py b/netbox/extras/models/models.py index 9cd778dbe..4bd257ccf 100644 --- a/netbox/extras/models/models.py +++ b/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 Name: Value. 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 header_safe filter to guard against HTTP header " + "injection, e.g. X-Object: {{ data.name | header_safe }}." ) ) 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 diff --git a/netbox/extras/tests/test_event_rules.py b/netbox/extras/tests/test_event_rules.py index 2764652d8..b5a185e55 100644 --- a/netbox/extras/tests/test_event_rules.py +++ b/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') diff --git a/netbox/extras/tests/test_models.py b/netbox/extras/tests/test_models.py index 0f7889ff0..296e7192d 100644 --- a/netbox/extras/tests/test_models.py +++ b/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. diff --git a/netbox/utilities/jinja2.py b/netbox/utilities/jinja2.py index 4263c69e2..620697007 100644 --- a/netbox/utilities/jinja2.py +++ b/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)