[Django] #17419: JSON template tag
Django
---------------------------------+-------------------------------
Reporter: lau | Owner: nobody
Type: New feature | Status: new
Component: Template system | Version: 1.3
Severity: Normal | Keywords: json template tag
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
---------------------------------+-------------------------------
It's nice to be able to quickly make Python lists and dictionaries into
JSON. The normal way of doing this is to dump the data structure as JSON
in the view and then outputting the JSON within a script tag in the
template, remembering to pipe it through safe. This little addition to
Django would streamline that process.
It lets you do this:
{{{
<script>
var data = {{ data|json }};
</script>
}}}
Instead of:
{{{
views.py:
from django.utils import simplejson
def home(request):
data = {'hello: ['world', 'universe']}
return render_to_response("home.html", {
'data': simplejson.dumps(data),
}, context_instance=get_context(request))
home.html:
<script>
var data = {{ data|safe }};
</script>
--
Ticket URL: <https://code.djangoproject.com/ticket/17419>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-----------------------------------+------------------------------------
Type: New feature | Status: new
Component: Template system | Version: 1.3
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by aaugustin):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:1>
Django
-----------------------------------+------------------------------------
Type: New feature | Status: new
Component: Template system | Version: 1.3
Keywords: json template tag | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+------------------------------------
Changes (by aaugustin):
* needs_docs: 0 => 1
* needs_tests: 0 => 1
* easy: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:2>
Django
-----------------------------------+------------------------------------
Type: New feature | Status: new
Component: Template system | Version: 1.3
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+------------------------------------
* cc: mike@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:3>
Django
-----------------------------------+---------------------------------------
Type: New feature | Status: new
Keywords: json template tag | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Changes (by carbonXT):
* needs_docs: 1 => 0
* version: 1.3 => 1.4-alpha-1
* needs_tests: 1 => 0
Comment:
I've uploaded a patch that applies cleanly to trunk that contains the new
filter, tests, and documentation.
Would be great to get this into 1.4 - let me know if you need any
additions/changes in order to accept it. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:4>
Django
-----------------------------------+---------------------------------------
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
-----------------------------------+---------------------------------------
* needs_better_patch: 0 => 1
Comment:
I don't believe marking the output as safe by default is the right thing
to do.
Not everyone adds CDATA markers to its <script> tags. Actually, most
frontend devs I've worked with don't.
I'd prefer `{{ data|json|safe }}` within CDATA sections and `{{ data|json
}}` everywhere else, because security should be on be default.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:5>
Django
-----------------------------------+---------------------------------------
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
-----------------------------------+---------------------------------------
* needs_better_patch: 1 => 0
Comment:
> Not everyone adds CDATA markers to its <script> tags.
Only required if you're using an XHTML doctype (rather than HTML 5 or
4.01). But no matter - I'm on board with a safety-by-default approach.
I've attached another patch, this one does not mark the output safe. Docs
and tests are updated as well. Thanks for reviewing.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:6>
Django
-----------------------------------+---------------------------------------
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+---------------------------------------
* owner: nobody => aaugustin
Comment:
Thanks for updating the patch.
This is a new feature, so I need to check if I can slip it in 1.4 now or
if it'll have to wait for 1.5. Anyway, I intend to commit it eventually,
as I've felt the need for such a tag quite often.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:7>
Django
-----------------------------------+---------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+---------------------------------------
Comment (by olau):
Sounds nice that it's going in, but playing it safe isn't necessarily that
helpful. {{{escape}}} turns " into {{{"}}}, which means you can't
output a string in that JSON without {{{safe}}} - including a simple
object like {{{ { "a": 123 }}}}.
I've tested a simple
{{{
<script>
var x = "foo";
</script>
}}}
with both an HTML5 doctype an XHTML 1 strict doctype, and it doesn't work
in any of them. So it seems to me this filter is never useful in script
tags without {{{safe}}}? It would perhaps be better to add a warning to
the documentation?
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Component: Template system | Version:
Keywords: json template tag | Resolution:
Has patch: 1 | Triage Stage: Ready for
Needs tests: 0 | checkin
Easy pickings: 1 | Needs documentation: 0
| Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by neaf):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:9>
Django
-----------------------------------+---------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+---------------------------------------
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Component: Template system | Version:
Keywords: json template tag | Resolution:
Has patch: 1 | Triage Stage: Ready for
Needs tests: 0 | checkin
| Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by cypreess):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:11>
Django
-----------------------------------+---------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+---------------------------------------
* stage: Ready for checkin => Accepted
Comment:
This ticket is assigned to me because I'm currently working on it... It's
recommended to ping the owner of a ticket before taking it over, to avoid
duplicate work...
This is a new feature and we're about to release 1.4 beta, so I can't
commit it now
Unless the other core devs have more objections than expected, I'll commit
it for 1.5.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:12>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-----------------------------------+-------------------------------------
* version: 1.4-alpha-1 => SVN
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Design
Has patch: 1 | decision needed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* easy: 1 => 0
* stage: Accepted => Design decision needed
Comment:
When the output isn't marked safe, indeed, `{{ foobar|json }}` will fail
whenever `foobar` contains strings. But I don't think Django can or should
do something about this fact. If the output were arbitrarily (and wrongly)
marked safe, that would defeat Django's anti-XSS protection.
Let's just try this with some interesting values of `a`:
{{{
<script>var a = {{ a|json }};</script>
}}}
For instance:
{{{
<script>var a =
"foo</script><script>alert('pwnd!');</script><script>bar;"</script>
}}}
Wrapping with CDATA doesn't help:
{{{
<script><![CDATA[var a =
"foo]]></script><script>alert('pwnd!');</script><script><![CDATA[bar;";]]></script>
}}}
Per the "Don't Give Users Guns Aimed At Feet", this isn't possible.
----
So I hesitate between two alternatives at this point:
- add the filter with safety-by-default, and explain in the docs that if
you have a problem with quotes being escaped, you should use another
technique — like loading the data via AJAX — or carefully escape all the
strings that end up in you data structure and add `|safe` in your
template.
- not add the filter at all, because the only implementation we can afford
(the safe one) isn't useful enough.
What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Design
Has patch: 1 | decision needed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by carbonXT):
It's a bit hacky, but we might be able to use JSONEncoder.iterencode to
escape only the string data in the json object. Proof of concept:
{{{
from django.utils import simplejson
from django.utils.html import escape
def encode_as_escaped_json(obj):
result = ''
for part in simplejson.JSONEncoder().iterencode(obj):
if part[0:3] == ', "':
result += ', "' + escape(part[3:-1]) + '"'
elif part[0] == '"':
result += '"' + escape(part[1:-1]) + '"'
else:
result += part
return result
if __name__ == '__main__':
my_obj = [
{'k1': '</script><script>Attack!</script><script>', 'k2': 42},
'e"eer',
'</script><script>More attack!</script><script>',
]
escaped_json = encode_as_escaped_json(my_obj)
print escaped_json
}}}
Running this yields:
{{{
1558]$ python ./tmp.py
[{"k2": 42, "k1":
"</script><script>Attack!</script><script>"},
"e\"eer", "</script><script>More
attack!</script><script>"]
}}}
This works because iterencode() returns string-based dictionary keys and
values as '''"<string>"''', and strings in lists as ''', "<string>"''' .
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:15>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Changes (by aaugustin):
* stage: Design decision needed => Accepted
Comment:
It's hard to prove that this technique is secure...
We could achieve a similar effect with a custom JSON encode that only
escapes strings. I'm going to write a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:16>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by aaugustin):
Custom JSON encoders will only be called if the default JSON encoder can't
process some data. Since the default encoder can process strings, custom
encoders don't resolve our problem.
I've also tried recursively escaping the value before passing it to the
JSON encoder (see attached patch), but it's still insecure :(
{{{
>>> from django.template import *
>>> Template('{{ data|json }}').render(Context({'data': '<>'}))
u'"<>"'
}}}
{{{
>>> class NastyInt(int):
... def __str__(self):
... return '</script><script>alert("%d");' % self
...
>>> Template('{{ data|json }}').render(Context({'data': NastyInt(42)}))
u'</script><script>alert("42");'
}}}
Of course, this is an edge case, but we can't compromise on security.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:17>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by aaugustin):
* status: new => closed
* resolution: => wontfix
Comment:
I've finally changed my mind on this ticket. There's a good reason why it
isn't recommended to just include raw JSON data in the HTML: it's
[https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet
extremely hard to prevent XSS] unless you enforce other rules about the
context, and this isn't under the control of Django.
It's obviously possible to solve the problem for particular use cases with
controlled data. In such cases, the implementation only takes a few lines
(see the first patch on this ticket). However, it appears very hard to
provide a general `|json` filter that will be safe in all contexts.
If you think you have a working implementation, please submit it on the
mailing list. I'll expect strong evidence that it's secure :)
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:18>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Type: New feature | Status: closed
Component: Template system | Version: master
Severity: Normal | Resolution: wontfix
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by jrief):
Sorry for adding my two cents to this ticket, 3 years after it has been
set to ''wontfix'', but for real projects such a filter still is an issue
and often required. And since there is no solution out-of-the-box,
programmers start to implement their own stuff, which then is vulnerable
to exactly the XSS attacks you're referring to. For instance here:
https://github.com/divio/django-
cms/blob/develop/cms/templatetags/cms_js_tags.py#L14.
Therefore I started to rethink about this problem and came to a solution
which seems to be secure; at least I was unable to inject malicious code
into this JSON filter. Please have a look at my attached implementation.
One thing to note here: If someone pushes data through this filter marked
as safe (using {{{mark_safe}}}), then of course XSS attacks are possible,
but this is intentional misbehavior if applied to non-validated content.
All other Python lists, dicts and strings (in my opinion) are safe when
pushed through this filter.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:19>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Type: New feature | Status: closed
Component: Template system | Version: master
Severity: Normal | Resolution: wontfix
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by abhillman):
Replying to [comment:18 aaugustin]:
> I've finally changed my mind on this ticket. There's a good reason why
it isn't recommended to just include raw JSON data in the HTML: it's
[https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet
extremely hard to prevent XSS] unless you enforce other rules about the
context, and this isn't under the control of Django.
>
> It's obviously possible to solve the problem for particular use cases
with controlled data. In such cases, the implementation only takes a few
lines (see the first patch on this ticket). However, it appears very hard
to provide a general `|json` filter that will be safe in all contexts.
>
> If you think you have a working implementation, please submit it on the
mailing list. I'll expect strong evidence that it's secure :)
This is a pretty old ticket. But out of curiosity, how would you deal with
this kind of situation, Aymeric? That is, binding some data to a
JavaScript variable. Just create an API endpoint to grab data via ajax and
bind it to a variable there? Create a template tag for your own usage
given that you are aware of its potential security issues?
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:20>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Type: New feature | Status: closed
Component: Template system | Version: master
Severity: Normal | Resolution: wontfix
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by aaugustin):
Yes, I would do the former if user-controlled data is involved and the
latter if the data is sufficiently simple and controlled that security
isn't an issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:21>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by timgraham):
* status: closed => new
* cc: apollo13 (added)
* has_patch: 1 => 0
* resolution: wontfix =>
Comment:
Florian says we should include this because, for one reason, there are
many insecure third-party implementations. He "got an okay from fusionbox
to pull [https://github.com/fusionbox/django-
argonauts/blob/master/argonauts/templatetags/argonauts.py their json
filter] into core."
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:22>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by aaugustin):
I agree. (I changed my mind on this.)
We should document clearly the safety restrictions.
If I understand correctly, that usage is unsafe:
{{{
<div data-json-stuff="{{ stuff | json }}">
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:23>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by apollo13):
Yes, this usage is unsafe since the filter first and foremost just
prevents "</script>" from getting executed.
As for mentioning the origial author(s), we should not forgot to clearly
attribute "Fusionbox, Inc. <rogra...@fusionbox.com>".
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:24>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by natevw):
The suggested patch has rather surprising behavior to me, with its custom
escaping. Normally an HTML attribute is the *best* place to put JSON-
formatted data:
- plays nicely most template language's default HTML escaping (i.e. the
usual `"` behaves correctly)
- provides a safe place for data to live _as data_, rather than as code
- just as easy to get to from an external script file as an inline one
- assuming it is placed in a `data-` attribute (as it should be), access
can still be very convenient:
{{{
<script type="x-data/x-config" id="variables" data-my-info="{{
info|to_json }}">
<script>
// with jQuery
var myInfo = $('#variables').data('myInfo');
// without, one simple option
var myInfo =
JSON.parse(document.getElementById('variables').getAttribute('data-my-
info'));
</script>
}}}
Obviously this is very different usage than intended by django-argonauts,
as evidenced by the examples [https://github.com/fusionbox/django-
argonauts/blob/593656e710b6aca3ca15368b2ca7cd1cae655b47/README.rst their
current README], which are using the output as JavaScript literals
directly within an inline script.
The argonauts approach isn't necessarily wrong, but feels more like its
intended to be used as "generate some JavaScript" tag rather than one for
"output JSON content [safely in an HTML context]". Granted, the former
(inline JavaScript) could very well be what many developers want, and the
latter (HTML-safe JSON) ends up being a bit of a pain anywhere *but* in an
attribute.
Hopefully my 2¢ on an alternative consideration is helpful feedback.
There's certainly some irony in proposing a solution that only works one
place, up against a solution that is convenient everywhere *except* that
same place where it happens to be unsafe. For me it was a tradeoff: fully
escaped output is _safe_ anywhere it ends up. Unfortunately it's only
_useful_ when extracted from an attribute value, but that's a habit I was
willing to settle on; I can use the same pattern across pretty much any
platform's template engine. YMMV.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:25>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+-------------------------------------
Comment (by apollo13):
Replying to [comment:25 natevw]:
> The suggested patch has rather surprising behavior to me, with its
custom escaping. Normally an HTML attribute is the *best* place to put
JSON-formatted data
I would not necessarily agree on *best*. `<script
type="application/json">` is just as valid as a data attribute and
requires way less escaping. Strictly speaking, if you are actually putting
a lot of small JSON snippets into data attributes, escaping takes up quite
a bit of space (And we've all learned by now that compressing the output
inside TLS can be dangerous). That aside, your argument is not actually
correct; depending on the HTML attribute you are using, different escaping
rules apply (think of all the js event attributes [onclick etc…]).
So in the end, we at least need documentation on how to safely use JSON in
HTML (though one might argue that docs exist already outside of Django and
this is not Django specific) and/or make usages outside of `data-*`
attributes also safe (ie the json filter for the most "common"
situations).
Regarding:
> Obviously this is very different usage than intended by django-
argonauts, as evidenced by the examples their current README, which are
using the output as JavaScript literals directly within an inline script.
Yes, this is imo an important example as loads and loads of tutorials on
the web suggest such a usage (for better or worse).
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:26>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+-------------------------------------
* cc: adrian.macneil@… (added)
Comment:
I think it's very important to add some form of `json` filter. Right now
there is a situation where developers are putting the output of
`json.dumps()` directly into templates and marking it as safe, which
causes an instant XSS vulnerability (I have seen this in the wild, hence
why I came across this ticket). This is a fairly complicated security
issue, so Django should make it easy for users to do the right thing.
To summarize previous comments, there are two different contexts where you
may wish to use JSON in templates:
1. Inside HTML (e.g. `data-*` attributes)
2. Inside CDATA (e.g. `var foo = {};` inside a `<script>` tag)
The problem here is that each context requires different escaping rules,
and there is no easy way for Django to tell which context you are in.
To escape for context 1 (inside html attributes) is actually fairly easy -
run `json.dumps()` on the object, then apply regular html escaping:
{{{
foo = {'foo': 'bar'}
json_str = json.dumps(foo)
}}}
{{{
<div data-foo="{{ json_str }}">
}}}
{{{
<div data-foo="{"foo":"bar"}">
}}}
To escape for context 2 (inside script tags), the problem is that HTML
escaping will produce invalid javascript:
{{{
<script>
var foo = {{ json_str }};
</script>
}}}
{{{
<script>
var foo = {"foo":"bar"}; // causes JS parse error
</script>
}}}
Currently people are solving this by marking the JSON string as safe,
incorrectly assuming that the output is safe inside a script tag. However,
as has been mentioned above, some valid JSON has special meaning inside
script tags:
{{{
foo = {'foo': 'bar </script> attack'}
json_str = json.dumps(foo)
}}}
{{{
<script>
var foo = {{ json_str|safe }};
</script>
}}}
{{{
<script>
var foo = {"foo":"bar </script> attack"};
</script>
}}}
The problem here is that HTML has no knowledge of parsing JS, so treats
the first `</script>` as the end of the script tag, and allows the
attacker to insert arbitrary HTML or JS afterwards.
This can be solved simply and effectively by replacing unsafe HTML
characters with unicode escape sequences. The characters which should be
replaced are `&`, `<`, `>`, `\u2028`, and `\u2029` (the last two are
treated as newlines by some javascript engines, which may allow an
attacker to begin a new javascript instruction - I haven't seen them
mentioned in this thread yet). These 5 characters are also replaced by the
[http://api.rubyonrails.org/classes/ERB/Util.html#method-c-json_escape
json_escape] filter in Rails, which has a similar purpose. Safe output
will look like this:
{{{
<script>
var foo = {"foo":"bar \u003c/script\u003e attack"};
</script>
}}}
Replacing these characters is easy to do, and valid in both contexts (html
and script tags), so they should always be replaced by any Django `json`
filter. I have created a simple implementation of a json filter which
replaces these characters here:
https://gist.github.com/amacneil/5af7cd0e934f5465b695
The last remaining issue is whether the output of this filter should be
marked as safe. The alternatives are:
* Automatically mark `json` filter output as safe. It will work as
expected inside script tags, but any use inside html attributes will
result in invalid HTML and potential XSS attacks.
* Do not mark output as safe. JSON will be correctly escaped inside html
attributes, and use inside script tags will result in malformed JS. Safe
use inside script tags can be achieved by using `{{ data|json|safe }}`.
Developers must be made aware that the `safe` filter should ONLY be used
when in the context of a script tag, and not for general HTML.
Neither of these is a great option, because it relies on the developer
knowing/remembering that output is safe in one context, but unsafe in
another. However, of the two I would probably prefer the second option
(JSON output is unsafe by default, and must be manually marked as safe for
use inside script tags), only because it is more safe by default, and
requires explicit thought to force the output as safe, which should ring
alarm bells and cause extra caution.
A final alternative may be to create another filter, named something like
`scriptjson`, which has the same effect as `json`, but which marks the
output as safe. Documentation could state that this filter should ONLY be
used inside `<script>` tags, and that the regular `json` filter should be
used everywhere else. Functionally this would be identical to using
`json|safe`, but it is more semantically obvious what the author is trying
to do, and it would be easier to document correct usage.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:27>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+-------------------------------------
Comment (by gavinwahl):
I want to point out that django-argonauts can be used both to generate
javascript code and to populate a `<script type="application/json"`
element. `<script>` tags have the same escaping rules for their content no
matter what type they are, so it doesn't matter to the json filter. I
would prefer that the django docs endorse `<script
type="application/javascript">` rather than data attributes.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:28>
Django
-----------------------------------+-------------------------------------
Reporter: lau | Owner: aaugustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+-------------------------------------
* cc: sergeimaertens@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:29>
Django
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Nathan Shafer):
* cc: nate-djangoproject@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:30>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "json_tags.py" added.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by blighj):
My Two cents on this.
Escaping json is tricky for all the reasons outlined above, is it in a
<script> is it in an attribute and will people know what to do with a
filter that seems to output as safe json, but turns out it is only in
certain circumstances.
I would propose that we have a simple filter that outputs the json in a
<script type="application/json"></script> block
{{{
{{ data|as_json_script:"id_data" }}
}}}
which would output
{{{
<script id="id_data" type="application/json">{"hello": ["world",
"universe"]}</script>
}}}
and which can then be read in by other javascript
{{{
var data = JSON.parse(document.getElementById("id_data").textContent)
}}}
or similar with jquery etc
This doesn't leave developers under any illusion that it could be used in
html attribute or within a script tag and has the added advantage that it
facilitates moving away from inline javascript which is better for
supporting good CSP policies.
Would be happy to put together a pull request if there was support for
this approach.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:31>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Florian Apolloner):
I do not like the name of the filter, but I do like the idea. Data
attributes don't require any special considerations if done properly and
we can suggest to not use json in normal <script> tags (which one should
not have either way due to CSP).
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:32>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by blighj):
The name I am in no way tied to, the little thinking I did on it was
trying to make it similar to form.as_p, etc
I was looking for a name that would be obvious that you should not use
this tag in the other scenarios in the thread, attributes and inline
javascript.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:33>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mateusz Jankowski):
Replying to [comment:31 blighj]:
> Would be happy to put together a pull request if there was support for
this approach.
Hi! Are you still planning to work on it? I could contribute with
extending your patch with tests, if you like.
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:34>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by blighj):
Replying to [comment:34 Mateusz Jankowski]:
A review would be great, here is the current commit for the patch
https://github.com/blighj/django/commit/af32bab8f9575cb968cd92931cec6f442bf31723
A general review would be good, plus specific things that should be looked
at would be
- naming the filter
- location of functions, I thought it useful to have the main function
available outside the template filter, so it could be reused and
repurposed by other projects, so I included it in utils/html.py. This
caused me issues with including DjangoJSONEncoder at the top of the file,
so I included it in the function. Which is bad right?
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:35>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Gavin Wahl):
Why are you trying to magically support both already-encoded and to-be-
encoded arguments? This leads to bugs. `js_json('"asdf"')` -- is that an
json-encoded `asdf`, or a string whose value really has quotes in it?
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:36>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by blighj):
Seemed like a good idea at the time. If you had a variable that was
already json it would give you a simple way to wrap it in a script block.
If for what ever reason you had the following string passed to your
template
'{"variable":"user contributed string</script>"}'
which you wanted to output to javascript in a similar style, the filter
would be useful to output in a script safely but keeping it as json
What I hadn't considered was your simple string example or equally, if
someone did to_json("4"). They will expect that to be output as a string
not a number.
So yeah let's get rid of that idea, if someone has a json string and want
to use this filter, then call json.loads first.
Any other comments or advice?
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:37>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Jonas Haag):
Removed the "magic string type check" and updated to latest master:
https://github.com/django/django/pull/9235
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:38>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:39>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: new
Component: Template system | Version: master
Severity: Normal | Resolution:
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:40>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Component: Template system | Version: master
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"8c709d79cbd1a7bb975f58090c17a1178a0efb80" 8c709d79]:
{{{
#!CommitTicketReference repository=""
revision="8c709d79cbd1a7bb975f58090c17a1178a0efb80"
Fixed #17419 -- Added json_tag template filter.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:41>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: closed
Component: Template system | Version: master
Severity: Normal | Resolution: fixed
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"02cd16a7a04529c726e5bb5a13d5979119f25c7d" 02cd16a]:
{{{
#!CommitTicketReference repository=""
revision="02cd16a7a04529c726e5bb5a13d5979119f25c7d"
Refs #17419 -- Removed IE8 support in json_script example.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:42>
Django
-------------------------------------+-------------------------------------
Reporter: Lau Bech Lauritzen | Owner: Aymeric
| Augustin
Type: New feature | Status: closed
Component: Template system | Version: master
Severity: Normal | Resolution: fixed
Keywords: json template tag | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"d5482dfe20cfd4e244db600733d8a5cf58664f2c" d5482df]:
{{{
#!CommitTicketReference repository=""
revision="d5482dfe20cfd4e244db600733d8a5cf58664f2c"
[2.1.x] Refs #17419 -- Removed IE8 support in json_script example.
Backport of 02cd16a7a04529c726e5bb5a13d5979119f25c7d from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17419#comment:43>