2
0
Эх сурвалжийг харах

Fixes #22079: Restrict environment_params to an allowlist of permitted keys

Replace JINJA_ENV_PARAMS_WITH_PATH_IMPORT with JINJA_ENV_PARAMS_ALLOWED,
a whitelist of permitted Jinja2 Environment parameters. Unknown keys are
rejected by clean() and stripped at render time.

The undefined parameter resolves values via direct class mapping instead
of import_string(). The finalize parameter is deprecated and blocked
from new use via clean(); existing stored values continue to resolve via
import_string() to preserve backward compatibility. No data migrations.
Jason Novinger 1 долоо хоног өмнө
parent
commit
d124c5fe86

+ 43 - 5
netbox/extras/constants.py

@@ -1,3 +1,5 @@
+from jinja2 import ChainableUndefined, DebugUndefined, StrictUndefined, Undefined
+
 from core.events import *
 from extras.choices import LogLevelChoices
 
@@ -32,11 +34,47 @@ WEBHOOK_EVENT_TYPES = {
     JOB_ERRORED: 'job_ended',
 }
 
-# Jinja environment parameters which support path imports
-JINJA_ENV_PARAMS_WITH_PATH_IMPORT = (
-    'undefined',
-    'finalize',
-)
+# Allowed Jinja2 environment parameters and their permitted values.
+# Only keys listed here may appear in a template's environment_params.
+#   None  = any JSON-serializable value accepted (scalars, booleans, etc.)
+#   dict  = only dict keys accepted; dict values are the resolved Python objects
+#
+# Note: 'finalize' is intentionally absent. It is deprecated and handled as a
+# legacy carve-out in RenderTemplateMixin (blocked from new use, but existing
+# stored values continue to resolve via import_string at render time).
+JINJA_ENV_PARAMS_ALLOWED = {
+    # Boolean / scalar params (accept any JSON-serializable value)
+    'auto_reload': None,
+    'autoescape': None,
+    'cache_size': None,
+    'enable_async': None,
+    'keep_trailing_newline': None,
+    'lstrip_blocks': None,
+    'optimized': None,
+    'trim_blocks': None,
+    # String params (template syntax delimiters)
+    'block_start_string': None,
+    'block_end_string': None,
+    'comment_start_string': None,
+    'comment_end_string': None,
+    'line_comment_prefix': None,
+    'line_statement_prefix': None,
+    'newline_sequence': None,
+    'variable_start_string': None,
+    'variable_end_string': None,
+    # Mapped params (value must be a key in the dict; resolved to the dict value)
+    'undefined': {
+        'jinja2.ChainableUndefined': ChainableUndefined,
+        'jinja2.DebugUndefined': DebugUndefined,
+        'jinja2.StrictUndefined': StrictUndefined,
+        'jinja2.Undefined': Undefined,
+    },
+    # Excluded (dangerous — accept callables or trigger imports):
+    #   'bytecode_cache' — accepts arbitrary object
+    #   'extensions'     — Jinja2 internally calls import_string() on string entries
+    #   'finalize'       — deprecated; legacy carve-out in RenderTemplateMixin
+    #   'loader'         — accepts arbitrary object
+}
 
 # Dashboard
 DEFAULT_DASHBOARD = [

+ 76 - 6
netbox/extras/models/mixins.py

@@ -4,6 +4,7 @@ import os
 import sys
 from collections import defaultdict
 
+from django.core.exceptions import ValidationError
 from django.core.files.storage import storages
 from django.db import models
 from django.http import HttpResponse
@@ -11,7 +12,7 @@ from django.utils.module_loading import import_string
 from django.utils.translation import gettext_lazy as _
 
 from core.models import ObjectType
-from extras.constants import DEFAULT_MIME_TYPE, JINJA_ENV_PARAMS_WITH_PATH_IMPORT
+from extras.constants import DEFAULT_MIME_TYPE, JINJA_ENV_PARAMS_ALLOWED
 from extras.utils import filename_from_model, filename_from_object
 from utilities.jinja2 import render_jinja2
 
@@ -134,15 +135,84 @@ class RenderTemplateMixin(models.Model):
 
         return _context
 
+    def clean(self):
+        super().clean()
+
+        params = self.environment_params or {}
+        for key, value in params.items():
+            # finalize is deprecated: block new use but preserve existing stored values
+            if key == 'finalize':
+                raise ValidationError({
+                    'environment_params': _(
+                        'The "{key}" parameter is deprecated and may not be set on new or modified templates.'
+                    ).format(key=key)
+                })
+            if key not in JINJA_ENV_PARAMS_ALLOWED:
+                raise ValidationError({
+                    'environment_params': _(
+                        '"{key}" is not a permitted Jinja2 environment parameter.'
+                    ).format(key=key)
+                })
+            allowed = JINJA_ENV_PARAMS_ALLOWED[key]
+            if type(allowed) is dict:
+                if value not in allowed:
+                    raise ValidationError({
+                        'environment_params': _(
+                            'Invalid value "{value}" for parameter "{key}". '
+                            'Allowed values are: {allowed}'
+                        ).format(
+                            value=value,
+                            key=key,
+                            allowed=', '.join(sorted(allowed.keys()))
+                        )
+                    })
+
+    @staticmethod
+    def _filter_environment_params(params):
+        """
+        Return a copy of params with only permitted keys. Keys not in the allowlist are
+        stripped, except 'finalize' which is a deprecated legacy carve-out.
+        """
+        return {
+            key: value for key, value in params.items()
+            if key in JINJA_ENV_PARAMS_ALLOWED or key == 'finalize'
+        }
+
+    @staticmethod
+    def _resolve_mapped_params(params):
+        """
+        Resolve allowlisted params that have a value mapping (e.g. undefined class names)
+        to their Python objects via direct dict lookup. Returns a new dict with resolved values;
+        unresolved params are passed through unchanged.
+        """
+        resolved = {}
+        for name, value in params.items():
+            allowed = JINJA_ENV_PARAMS_ALLOWED.get(name)
+            if type(allowed) is dict and value in allowed:
+                resolved[name] = allowed[value]
+            else:
+                resolved[name] = value
+        return resolved
+
+    @staticmethod
+    def _resolve_finalize(params):
+        """
+        Legacy carve-out: resolve the deprecated 'finalize' parameter via import_string().
+        Existing templates with finalize continue to work; new use is blocked by clean().
+        """
+        if 'finalize' in params and type(params['finalize']) is str:
+            return {**params, 'finalize': import_string(params['finalize'])}
+        return params
+
     def get_environment_params(self):
         """
-        Pre-processing of any defined Jinja environment parameters (e.g. to support path resolution).
+        Pre-processing of any defined Jinja environment parameters.
         """
-        # Shallow-copy so resolved imports don't replace the string values on the model field itself.
+        # Shallow-copy so resolved values don't replace the strings on the model field.
         params = dict(self.environment_params or {})
-        for name, value in params.items():
-            if name in JINJA_ENV_PARAMS_WITH_PATH_IMPORT and type(value) is str:
-                params[name] = import_string(value)
+        params = self._filter_environment_params(params)
+        params = self._resolve_mapped_params(params)
+        params = self._resolve_finalize(params)
         return params
 
     def render(self, context=None, queryset=None):

+ 203 - 2
netbox/extras/tests/test_models.py

@@ -10,7 +10,7 @@ from django.core.files.storage import Storage
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.forms import ValidationError
 from django.test import TestCase, tag
-from jinja2 import StrictUndefined, TemplateError, TemplateSyntaxError, UndefinedError
+from jinja2 import DebugUndefined, StrictUndefined, TemplateError, TemplateSyntaxError, UndefinedError
 from PIL import Image
 
 from core.events import OBJECT_CREATED
@@ -27,6 +27,7 @@ from extras.models import (
     Tag,
     TaggedItem,
 )
+from extras.models.mixins import RenderTemplateMixin
 from tenancy.models import Tenant, TenantGroup
 from utilities.exceptions import AbortRequest
 from utilities.jinja2 import render_jinja2
@@ -1032,7 +1033,11 @@ class RenderTemplateMixinRenderTest(TestCase):
         with self.assertRaises(UndefinedError):
             strict.render({})
 
-    def test_environment_params_finalize_path_import(self):
+    def test_environment_params_finalize_legacy_resolution(self):
+        """
+        Existing finalize values continue to resolve via import_string() as a
+        legacy carve-out (CVE-2026-29514). New use is blocked by clean().
+        """
         t = ConfigTemplate(
             name='finalize',
             template_code='{{ v }}',
@@ -1180,3 +1185,199 @@ class EventRuleTestCase(TestCase):
             with self.assertRaises(ValidationError) as cm:
                 rule.clean()
             self.assertIn('action_data', cm.exception.message_dict)
+
+
+class JinjaEnvironmentParamsCleanTest(TestCase):
+    """Tests for RenderTemplateMixin.clean() validation of environment_params."""
+
+    def _make_template(self, environment_params):
+        return ConfigTemplate(
+            name='test',
+            template_code='{{ "test" }}',
+            environment_params=environment_params,
+        )
+
+    def test_allowed_scalar_params_pass(self):
+        template = self._make_template({'trim_blocks': True, 'lstrip_blocks': True})
+        template.clean()
+
+    def test_autoescape_boolean_passes(self):
+        template = self._make_template({'autoescape': True})
+        template.clean()
+
+    def test_valid_undefined_passes(self):
+        for value in (
+            'jinja2.Undefined',
+            'jinja2.ChainableUndefined',
+            'jinja2.DebugUndefined',
+            'jinja2.StrictUndefined',
+        ):
+            template = self._make_template({'undefined': value})
+            template.clean()
+
+    def test_invalid_undefined_rejected(self):
+        template = self._make_template({'undefined': 'subprocess.getoutput'})
+        with self.assertRaises(ValidationError) as cm:
+            template.clean()
+        self.assertIn('environment_params', cm.exception.message_dict)
+
+    def test_unknown_key_rejected(self):
+        template = self._make_template({'extensions': ['os']})
+        with self.assertRaises(ValidationError) as cm:
+            template.clean()
+        self.assertIn('environment_params', cm.exception.message_dict)
+
+    def test_finalize_blocked_from_new_use(self):
+        template = self._make_template({'finalize': 'subprocess.getoutput'})
+        with self.assertRaises(ValidationError) as cm:
+            template.clean()
+        self.assertIn('environment_params', cm.exception.message_dict)
+
+    def test_empty_params_pass(self):
+        template = self._make_template({})
+        template.clean()
+
+    def test_none_params_pass(self):
+        template = self._make_template(None)
+        template.clean()
+
+    def test_exporttemplate_clean_rejects_unknown_key(self):
+        """MRO smoke test: ExportTemplate.clean() reaches RenderTemplateMixin.clean()."""
+        obj = ExportTemplate(
+            name='test',
+            template_code='{{ "test" }}',
+            environment_params={'loader': 'some.loader'},
+        )
+        with self.assertRaises(ValidationError) as cm:
+            obj.clean()
+        self.assertIn('environment_params', cm.exception.message_dict)
+
+    def test_configtemplate_clean_rejects_finalize(self):
+        """MRO smoke test: ConfigTemplate.clean() reaches RenderTemplateMixin.clean()."""
+        obj = ConfigTemplate(
+            name='test',
+            template_code='{{ "test" }}',
+            environment_params={'finalize': 'subprocess.getoutput'},
+        )
+        with self.assertRaises(ValidationError) as cm:
+            obj.clean()
+        self.assertIn('environment_params', cm.exception.message_dict)
+
+
+class JinjaEnvironmentParamsFilterTest(TestCase):
+    """Tests for RenderTemplateMixin._filter_environment_params()."""
+
+    def test_allowed_keys_pass_through(self):
+        params = {'trim_blocks': True, 'autoescape': False}
+        result = RenderTemplateMixin._filter_environment_params(params)
+        self.assertEqual(result, params)
+
+    def test_unknown_keys_stripped(self):
+        params = {'extensions': ['os'], 'loader': 'x', 'trim_blocks': True}
+        result = RenderTemplateMixin._filter_environment_params(params)
+        self.assertEqual(result, {'trim_blocks': True})
+
+    def test_finalize_preserved_as_legacy(self):
+        params = {'finalize': 'some.module.func', 'trim_blocks': True}
+        result = RenderTemplateMixin._filter_environment_params(params)
+        self.assertEqual(result, params)
+
+    def test_empty_params(self):
+        self.assertEqual(RenderTemplateMixin._filter_environment_params({}), {})
+
+
+class JinjaEnvironmentParamsResolveTest(TestCase):
+    """Tests for RenderTemplateMixin._resolve_mapped_params()."""
+
+    def test_undefined_resolved_to_class(self):
+        params = {'undefined': 'jinja2.StrictUndefined'}
+        result = RenderTemplateMixin._resolve_mapped_params(params)
+        self.assertIs(result['undefined'], StrictUndefined)
+
+    def test_unrecognized_undefined_value_passed_through(self):
+        params = {'undefined': 'not.a.real.class'}
+        result = RenderTemplateMixin._resolve_mapped_params(params)
+        self.assertEqual(result['undefined'], 'not.a.real.class')
+
+    def test_scalar_params_passed_through(self):
+        params = {'trim_blocks': True, 'autoescape': False}
+        result = RenderTemplateMixin._resolve_mapped_params(params)
+        self.assertEqual(result, params)
+
+    def test_empty_params(self):
+        self.assertEqual(RenderTemplateMixin._resolve_mapped_params({}), {})
+
+
+class JinjaEnvironmentParamsFinalizeTest(TestCase):
+    """Tests for RenderTemplateMixin._resolve_finalize() legacy carve-out."""
+
+    def test_finalize_string_resolved_via_import_string(self):
+        params = {'finalize': 'extras.tests.test_models.finalize_none_to_dash'}
+        result = RenderTemplateMixin._resolve_finalize(params)
+        self.assertIs(result['finalize'], finalize_none_to_dash)
+
+    def test_finalize_non_string_passed_through(self):
+        params = {'finalize': 42}
+        result = RenderTemplateMixin._resolve_finalize(params)
+        self.assertEqual(result['finalize'], 42)
+
+    def test_no_finalize_key_unchanged(self):
+        params = {'trim_blocks': True}
+        result = RenderTemplateMixin._resolve_finalize(params)
+        self.assertEqual(result, {'trim_blocks': True})
+
+    def test_invalid_import_path_raises_import_error(self):
+        params = {'finalize': 'nonexistent.module.func'}
+        with self.assertRaises(ImportError):
+            RenderTemplateMixin._resolve_finalize(params)
+
+    def test_empty_params(self):
+        self.assertEqual(RenderTemplateMixin._resolve_finalize({}), {})
+
+
+class JinjaEnvironmentParamsIntegrationTest(TestCase):
+    """Integration tests for get_environment_params() end-to-end."""
+
+    def _make_template(self, environment_params):
+        return ConfigTemplate(
+            name='test',
+            template_code='{{ "test" }}',
+            environment_params=environment_params,
+        )
+
+    def test_full_pipeline_with_undefined(self):
+        template = self._make_template({'undefined': 'jinja2.StrictUndefined', 'trim_blocks': True})
+        params = template.get_environment_params()
+        self.assertIs(params['undefined'], StrictUndefined)
+        self.assertIs(params['trim_blocks'], True)
+
+    def test_full_pipeline_strips_unknown_and_resolves(self):
+        template = self._make_template({
+            'extensions': ['os'],
+            'undefined': 'jinja2.DebugUndefined',
+            'trim_blocks': True,
+        })
+        params = template.get_environment_params()
+        self.assertNotIn('extensions', params)
+        self.assertIs(params['undefined'], DebugUndefined)
+        self.assertIs(params['trim_blocks'], True)
+
+    def test_full_pipeline_finalize_resolves(self):
+        template = self._make_template({
+            'finalize': 'extras.tests.test_models.finalize_none_to_dash',
+        })
+        params = template.get_environment_params()
+        self.assertIs(params['finalize'], finalize_none_to_dash)
+
+    def test_does_not_mutate_stored_value(self):
+        template = self._make_template({'undefined': 'jinja2.StrictUndefined'})
+        template.get_environment_params()
+        self.assertEqual(template.environment_params['undefined'], 'jinja2.StrictUndefined')
+
+    def test_none_environment_params(self):
+        template = self._make_template(None)
+        self.assertEqual(template.get_environment_params(), {})
+
+    def test_empty_environment_params(self):
+        template = self._make_template({})
+        self.assertEqual(template.get_environment_params(), {})