From d655030770081e2dfe46f90e27620472a502289d Mon Sep 17 00:00:00 2001 From: David Lord Date: Thu, 2 May 2024 09:14:00 -0700 Subject: [PATCH] disallow invalid characters in keys to xmlattr filter --- CHANGES.rst | 6 ++++++ src/jinja2/filters.py | 22 +++++++++++++++++----- tests/test_filters.py | 11 ++++++----- 3 files changed, 29 insertions(+), 10 deletions(-) Index: Jinja2-3.1.2/CHANGES.rst =================================================================== --- Jinja2-3.1.2.orig/CHANGES.rst +++ Jinja2-3.1.2/CHANGES.rst @@ -9,6 +9,12 @@ Released 2022-04-28 :issue:`1645` - Handle race condition in ``FileSystemBytecodeCache``. :issue:`1654` +- The ``xmlattr`` filter does not allow keys with ``/`` solidus, ``>`` + greater-than sign, or ``=`` equals sign, in addition to disallowing spaces. + Regardless of any validation done by Jinja, user input should never be used + as keys to this filter, or must be separately validated first. + GHSA-h75v-3vvj-5mfj + Version 3.1.1 ------------- Index: Jinja2-3.1.2/src/jinja2/filters.py =================================================================== --- Jinja2-3.1.2.orig/src/jinja2/filters.py +++ Jinja2-3.1.2/src/jinja2/filters.py @@ -248,13 +248,25 @@ def do_items(value: t.Union[t.Mapping[K, yield from value.items() +# Check for characters that would move the parser state from key to value. +# https://html.spec.whatwg.org/#attribute-name-state +_attr_key_re = re.compile(r"[\s/>=]", flags=re.ASCII) + + @pass_eval_context def do_xmlattr( eval_ctx: "EvalContext", d: t.Mapping[str, t.Any], autospace: bool = True ) -> str: """Create an SGML/XML attribute string based on the items in a dict. - All values that are neither `none` nor `undefined` are automatically - escaped: + + **Values** that are neither ``none`` nor ``undefined`` are automatically + escaped, safely allowing untrusted user input. + + User input should not be used as **keys** to this filter. If any key + contains a space, ``/`` solidus, ``>`` greater-than sign, or ``=`` equals + sign, this fails with a ``ValueError``. Regardless of this, user input + should never be used as keys to this filter, or must be separately validated + first. .. sourcecode:: html+jinja @@ -273,12 +285,23 @@ def do_xmlattr( As you can see it automatically prepends a space in front of the item if the filter returned something unless the second parameter is false. + + Keys with ``/`` solidus, ``>`` greater-than sign, or ``=`` equals sign + are not allowed. + + Keys with spaces are not allowed. """ - rv = " ".join( - f'{escape(key)}="{escape(value)}"' - for key, value in d.items() - if value is not None and not isinstance(value, Undefined) - ) + items = [] + for key, value in d.items(): + if value is None or isinstance(value, Undefined): + continue + + if _attr_key_re.search(key) is not None: + raise ValueError("Invalid character in attribute name: {key!r}") + + items.append(f'{escape(key)}="{escape(value)}"') + + rv = " ".join(items) if autospace and rv: rv = " " + rv Index: Jinja2-3.1.2/tests/test_filters.py =================================================================== --- Jinja2-3.1.2.orig/tests/test_filters.py +++ Jinja2-3.1.2/tests/test_filters.py @@ -871,3 +871,10 @@ class TestFilter: with pytest.raises(TemplateRuntimeError, match="No filter named 'f'"): t1.render(x=42) t2.render(x=42) + + @pytest.mark.parametrize("sep", ("\t", "\n", "\f", " ", "/", ">", "=")) + def test_xmlattr_key_invalid(self, env: Environment, sep: str) -> None: + with pytest.raises(ValueError, match="Invalid character"): + env.from_string("{{ {key: 'my_class'}|xmlattr }}").render( + key=f"class{sep}onclick=alert(1)" + )