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.