Hi all,
Django's code base has quite a few instances of string interpolation
being used to build up HTML e.g.:
contrib/admin/util.py
return mark_safe('%s: <a href="%s">%s</a>' %
(escape(capfirst(opts.verbose_name)),
admin_url,
escape(obj)))
The problem with this is that it is easy to forget to use 'escape',
giving rise to an XSS vulnerability. This has led to at least one
instance in the past:
https://github.com/django/django/commit/9f6d50d02e
(I couldn't find others, there might be. I'm sure there could be many in
3rd party code - there was at least one instance in some code I wrote).
It also makes review hard. In the above fragment, is it still vulnerable
to XSS due to the unescaped 'admin_url'? (AFAICS after looking at it,
'no', but it's not that easy to tell).
I'm going to clean these up using the following pattern:
return html_fragment('%s: <a href="%s">%s</a>',
capfirst(opts.verbose_name),
admin_url,
obj)
The implementation, which will be added to django.utils.html, is just:
def html_fragment(template, *args):
return mark_safe(template % tuple(map(conditional_escape, args)))
This is an all round win:
* It is secure by default, like autoescaping in templates
* It is easier to use - you only have to import 'html_fragment', instead
of both 'mark_safe' and 'escape'
* The code is easier to read due to fewer parenthesis
* It makes XSS problems more greppable. You now have to grep for:
* all string interpolations using % (which should be removed and
replaced with html_fragment if the result is eventually
mark_safe'd).
As before, you have to audit:
* mark_safe (but this will have far fewer instances now)
* |safe
* {% autoescape off %}
This utility will be public and documented, and suggested as an
alternative to ever using 'mark_safe' or 'escape' directly.
Apart from letting people know, I've got a few reasons for emailing
about this:
1) Are there are any better alternatives? I'm ruling out use of
'Template' because it is overly verbose for this use case, both in
API and template syntax.
2) Any better name than 'html_fragment'?
3) It made me realise we should have thought of this earlier, and
that our security procedures need improving in this regard.
When we had the XSS exploit I mentioned above, we simply fixed
it by applying 'escape'. We didn't ask "why did this happen?
What is the root cause? How can we stop it ever happening again?"
We need to be much more rigorous about applying things like
the "5 Whys" http://en.wikipedia.org/wiki/5_Whys
especially for security problems.
Regards,
Luke
--
Parenthetical remarks (however relevant) are unnecessary
Luke Plant || http://lukeplant.me.uk/
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
