Opened 18 months ago

Last modified 14 months ago

#34581 assigned Cleanup/optimization

Filters should not implicitly mark unsafe strings as safe without escaping — at Initial Version

Reported by: Shai Berger Owned by: nobody
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Consider the following template code snippet:

{% autoescape off %}
{{ some_list|join:some_var }}
{% endautoescape %}

As noted in #34574, the current implementation joins the members of some_list without escaping them, but marks the end result as safe (since #34578, the joiner some_var is not escaped either -- but in most common use, it is a literal in the template, so it is safe to begin with).

Given that we already have a safeseq filter, and #34577 is introducing escapeseq as well, I think we should change the behavior of join and other related filters (an initial list that was given in the related discussion in the security team includes linenumbers, linebreaks, and linebreaksbr, but there are probably others) so they don't mark unsafe strings as safe without escaping them.

The details change by the specific filter: The join filter may output an unsafe string when all of its inputs are unsafe. The linebreaks filter cannot -- it introduces HTML tags into its output, so this output must be marked safe. The join filter, too, should preserve safety -- so if some input (in the sequence or the joiner) is safe, again, the output must be safe.

When a filter gets an unsafe input, must produce a safe output, and autoescape is on, there's no real problem -- the input can be escaped. This is current behavior, mostly (join escapes input and marks the output safe, even if it isn't forced to).

But when autoescaping is off, inputs are not escaped -- however, the output, which usually includes their content verbatim, is still marked safe.

I suggest a general rule: A filter which gets unsafe input, should not escape it, and yet needs to produce safe output, should error out. This is a hardening, not a security fix, so it should follow a normal deprecation cycle: The actual change should only happen after the next LTS release.

This would affect many filters, and is backwards-incompatible, but I think it would make the escaping more consistent and predictable, and it would make Django more safe-by-default for some less-common use-cases.

Change History (0)

Note: See TracTickets for help on using tickets.
Back to Top