Commit aef2a0ec authored by Luke Plant's avatar Luke Plant Committed by Tim Graham
Browse files

Fixed #25018 -- Changed simple_tag to apply conditional_escape() to its output.

This is a security hardening fix to help prevent XSS (and incorrect HTML)
for the common use case of simple_tag.

Thanks to Tim Graham for the review.
parent 9ed82154
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ from importlib import import_module

from django.utils import six
from django.utils.deprecation import RemovedInDjango20Warning
from django.utils.html import conditional_escape
from django.utils.inspect import getargspec
from django.utils.itercompat import is_iterable

@@ -201,6 +202,8 @@ class SimpleNode(TagHelperNode):
        if self.target_var is not None:
            context[self.target_var] = output
            return ''
        if context.autoescape:
            output = conditional_escape(output)
        return output


+22 −2
Original line number Diff line number Diff line
@@ -441,6 +441,22 @@ A few things to note about the ``simple_tag`` helper function:
* If the argument was a template variable, our function is passed the
  current value of the variable, not the variable itself.

Unlike other tag utilities, ``simple_tag`` passes its output through
:func:`~django.utils.html.conditional_escape` if the template context is in
autoescape mode, to ensure correct HTML and protect you from XSS
vulnerabilities.

If additional escaping is not desired, you will need to use
:func:`~django.utils.safestring.mark_safe` if you are absolutely sure that your
code does not contain XSS vulnerabilities. For building small HTML snippets,
use of :func:`~django.utils.html.format_html` instead of ``mark_safe()`` is
strongly recommended.

.. versionchanged:: 1.9

   Auto-escaping for ``simple_tag`` as described in the previous two paragraphs
   was added.

If your template tag needs to access the current context, you can use the
``takes_context`` argument when registering your tag::

@@ -792,12 +808,16 @@ Ultimately, this decoupling of compilation and rendering results in an
efficient template system, because a template can render multiple contexts
without having to be parsed multiple times.

.. _tags-auto-escaping:

Auto-escaping considerations
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The output from template tags is **not** automatically run through the
auto-escaping filters. However, there are still a couple of things you should
keep in mind when writing a template tag.
auto-escaping filters (with the exception of
:meth:`~django.template.Library.simple_tag` as described above). However, there
are still a couple of things you should keep in mind when writing a template
tag.

If the ``render()`` function of your template stores the result in a context
variable (rather than returning the result in a string), it should take care
+43 −0
Original line number Diff line number Diff line
@@ -684,6 +684,49 @@ define built-in libraries via the ``'builtins'`` key of :setting:`OPTIONS
<TEMPLATES-OPTIONS>` when defining a
:class:`~django.template.backends.django.DjangoTemplates` backend.

.. _simple-tag-conditional-escape-fix:

``simple_tag`` now wraps tag output in ``conditional_escape``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In general, template tags do not autoescape their contents, and this behavior is
:ref:`documented <tags-auto-escaping>`. For tags like
:class:`~django.template.Library.inclusion_tag`, this is not a problem because
the included template will perform autoescaping. For
:class:`~django.template.Library.assignment_tag`, the output will be escaped
when it is used as a variable in the template.

For the intended use cases of :class:`~django.template.Library.simple_tag`,
however, it is very easy to end up with incorrect HTML and possibly an XSS
exploit. For example::

    @register.simple_tag(takes_context=True)
    def greeting(context):
        return "Hello {0}!".format(context['request'].user.first_name)

In older versions of Django, this will be an XSS issue because
``user.first_name`` is not escaped.

In Django 1.9, this is fixed: if the template context has ``autoescape=True``
set (the default), then ``simple_tag`` will wrap the output of the tag function
with :func:`~django.utils.html.conditional_escape`.

To fix your ``simple_tag``\s, it is best to apply the following practices:

* Any code that generates HTML should use either the template system or
  :func:`~django.utils.html.format_html`.

* If the output of a ``simple_tag`` needs escaping, use
  :func:`~django.utils.html.escape` or
  :func:`~django.utils.html.conditional_escape`.

* If you are absolutely certain that you are outputting HTML from a trusted
  source (e.g. a CMS field that stores HTML entered by admins), you can mark it
  as such using :func:`~django.utils.safestring.mark_safe`.

Tags that follow these rules will be correct and safe whether they are run on
Django 1.9+ or earlier.

Miscellaneous
~~~~~~~~~~~~~

+1 −1
Original line number Diff line number Diff line
@@ -2434,7 +2434,7 @@ class AdminViewStringPrimaryKeyTest(TestCase):
        expected_link = reverse('admin:%s_modelwithstringprimarykey_history' %
            ModelWithStringPrimaryKey._meta.app_label,
            args=(quote(self.pk),))
        self.assertContains(response, '<a href="%s" class="historylink"' % expected_link)
        self.assertContains(response, '<a href="%s" class="historylink"' % escape(expected_link))

    def test_redirect_on_add_view_continue_button(self):
        """As soon as an object is added using "Save and continue editing"
+19 −0
Original line number Diff line number Diff line
@@ -4,6 +4,7 @@ import warnings
from django import template
from django.template.defaultfilters import stringfilter
from django.utils import six
from django.utils.html import escape, format_html

register = template.Library()

@@ -109,6 +110,24 @@ def simple_tag_without_context_parameter(arg):
simple_tag_without_context_parameter.anything = "Expected simple_tag_without_context_parameter __dict__"


@register.simple_tag(takes_context=True)
def escape_naive(context):
    """A tag that doesn't even think about escaping issues"""
    return "Hello {0}!".format(context['name'])


@register.simple_tag(takes_context=True)
def escape_explicit(context):
    """A tag that uses escape explicitly"""
    return escape("Hello {0}!".format(context['name']))


@register.simple_tag(takes_context=True)
def escape_format_html(context):
    """A tag that uses format_html"""
    return format_html("Hello {0}!", context['name'])


@register.simple_tag(takes_context=True)
def current_app(context):
    return "%s" % context.current_app
Loading