Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#15365 closed (fixed)

Stronger warning for markup template filters

Reported by: db.pub.mail@… Owned by: nobody
Component: Documentation Version: 1.2
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

if the restructuredtext markup is meant to be safe (I cannot see anything
to say if it is or isn't... :/)

then the following demonstrates a potential issue:

thingy/lol.html

{% load markup %}

{{LOL|restructuredtext}}

views.py
...

def index(request):

return render_to_response('thingy/lol.html', {'LOL: "`NotMe

<javascript:alert(1)>`_"})

Change History (5)

comment:1 Changed 4 years ago by russellm

  • Component changed from Contrib apps to Documentation
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Firstly, if you even *suspect* that you have found a potential issue, *PLEASE* don't report it through Trac. Mail security@… with full details of the problem you think you have found.

As for the ticket itself:

The output of markup filters *must* be marked as safe strings -- otherwise, you wouldn't be able to print the HTML markup that they produce.

Whether the input has itself been sanitized is entirely up to your application.

I'm not entirely sure what you expect Django to be able to do in the case of the example you provide. The input isn't HTML, so there's nothing that Django could identify as requiring an escape. We can't automatically escape the < and >, because that would render the markup invalid, and reST would fail. Ultimately, you've provided correctly marked up input that will be interpreted as,

I think the best we can hope to do here is beef up the documentation warning of the potential hazards associated with using markup.

comment:2 Changed 4 years ago by db.pub.mail@…

Perhaps django could ensure that http:// (or /) is at the front of all links produced? (while this could break javascript links - I doubt users of markup will see this as a potential loss).
I haven't looked into the markup much at all to know if this would even be worth considering (if there are other 'features' of markup which are 'potential issues'). I suspect it wouldn't be... (but I could be wrong).

comment:3 Changed 4 years ago by gabrielhurley

  • milestone 1.3 deleted
  • Summary changed from if the restructuredtext markup is meant to be safe (I cannot see anything to say if it is or isn't... :/) ... to Stronger warning for markup template filters

I'd accept an admonition on ref/contrib/markup as an acceptable resolution to this ticket. We should point out that the output will be marked safe and unescaped.

There's no way for the markup filters to fulfill their intended purpose without producing unescaped results. These filters are sufficiently non-obvious that a note is worthwhile, but at some point there is no reasonable protection against not understanding the impact of template filters on secure application design.

Filtering on http:// protocols or paths under the site root can be exploited just as easily; there's no protection there.

IMHO, developers *must* be aware of trusting user input, and as long as we make obvious the fact that these filters will produce unescaped results we've done as much as we can do without crippling functionality. Especially with reST, darn near anything could be valid markup within a chunk of text. There's no rational filtering that can be applied to make it safe.

Let's be clear in our limitations here and leave it to developers to use power responsibly.

comment:4 Changed 4 years ago by gabrielhurley

  • Resolution set to fixed
  • Status changed from new to closed

In [15673]:

Fixed #15365 -- Added a warning to the contrib.markup docs reminding users that the marked up output will not be escaped.

comment:5 Changed 4 years ago by gabrielhurley

In [15674]:

[1.2.X] Fixed #15365 -- Added a warning to the contrib.markup docs reminding users that the marked up output will not be escaped.

Backport of [15673] from trunk.

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