#34581: Filters should not implicitly mark unsafe strings as safe without
escaping
-------------------------------------+-------------------------------------
Reporter: Shai Berger | Owner:
Type: | omerimzali
Cleanup/optimization | Status: assigned
Component: Template system | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old 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.
New 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 fulfills these three conditions,
should error out:
- it needs to produce safe output
- some of its input is unsafe,
- the context is such that it should not escape the input
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.
--
Comment (by Shai Berger):
Replying to [comment:5 omerimzali]:
>
> {{{
> "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."
> }}}
>
> A filter which gets unsafe input should not escape it. I totally agree
but do we mean it only when autoescape off? {% autoescape off %}
>
I guess this phrasing was unclear, and I changed it in hope it is clearer
now: The "should not change it" part is not part of "how a filter should
generally behave", but a further condition. A filter for which all three
conditions are true, should error out. "autoescape off" means that
filters, generally, should not escape the input.
> How should I check "All filters which can get unsafe input". I know
Django at least has 4 of them below. What's the right way to find others?
Should this patch contains all of them.
>
> {{{
> join
> linenumbers,
> linebreaks,
> linebreaksbr,
> }}}
>
All filters can get unsafe input. Whether the filter actually got safe or
unsafe input is only decided at runtime, and cannot be known in advance.
The interesting question is, "does the filter need to produce safe
output", and this changes from filter to filter: The last three you listed
always need to produce safe output, because they need to include HTML to
break lines; but for some filters, like {{{join}}}, this too is only
decided at runtime -- {{{join}}} needs to produce safe output if (and only
if) any of its inputs (the list members and the joiner) are safe. I can't
think of any way to find which filters are relevant (and when), except to
go over them one by one.
I hope this clarifies the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/34581#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/0107018aeb00e42c-9d47d8dc-601e-4766-9b81-881cc8eb0c55-000000%40eu-central-1.amazonses.com.
