id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
34581	Filters should not implicitly mark unsafe strings as safe without escaping	Shai Berger	nobody	"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.

"	Cleanup/optimization	new	Template system	dev	Normal				Unreviewed	0	0	0	0	0	0