Code

Opened 3 years ago

Closed 2 years ago

#17419 closed New feature (wontfix)

JSON template tag

Reported by: lau Owned by: aaugustin
Component: Template system Version: master
Severity: Normal Keywords: json template tag
Cc: mike@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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>

Attachments (6)

json.py (222 bytes) - added by lau 3 years ago.
JSON template tag
json-filter.diff (3.8 KB) - added by carbonXT 3 years ago.
json template filter with tests and docs
json-filter.v2.diff (3.7 KB) - added by carbonXT 2 years ago.
json template filter with tests and docs - output no longer marked safe
json-filter.v3.diff (3.8 KB) - added by neaf 2 years ago.
Add a warning about naively displaying strings from JSON marked as safe.
json-filter.v4.diff (2.2 KB) - added by neaf 2 years ago.
Add test for invalid argument (not JSON serializable)
17419-insecure-do-not-use-this.patch (6.5 KB) - added by aaugustin 2 years ago.

Download all attachments as: .zip

Change History (24)

Changed 3 years ago by lau

JSON template tag

comment:1 Changed 3 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by aaugustin

  • Easy pickings set
  • Needs documentation set
  • Needs tests set

comment:3 Changed 3 years ago by carbonXT

  • Cc mike@… added

comment:4 Changed 3 years ago by carbonXT

  • Needs documentation unset
  • Needs tests unset
  • Version changed from 1.3 to 1.4-alpha-1

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.

Changed 3 years ago by carbonXT

json template filter with tests and docs

comment:5 Changed 3 years ago by aaugustin

  • Patch needs improvement set

It isn't obvious to me that 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.

Wouldn't the current implementation break HTML parsing when the filter is used naively?

If so, I'd prefer {{ data|json|safe }} within CDATA sections and {{ data|json }} everywhere else -- security should be on be default.

Last edited 3 years ago by aaugustin (previous) (diff)

Changed 2 years ago by carbonXT

json template filter with tests and docs - output no longer marked safe

comment:6 Changed 2 years ago by carbonXT

  • Patch needs improvement unset

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.

comment:7 Changed 2 years ago by aaugustin

  • Owner changed from nobody to aaugustin

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.

comment:8 Changed 2 years ago by olau

Sounds nice that it's going in, but playing it safe isn't necessarily that helpful. escape turns " into &quot;, 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 = &quot;foo&quot;;
</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?

Changed 2 years ago by neaf

Add a warning about naively displaying strings from JSON marked as safe.

comment:9 Changed 2 years ago by neaf

  • Triage Stage changed from Accepted to Ready for checkin

comment:10 Changed 2 years ago by neaf

  • Triage Stage changed from Ready for checkin to Accepted

Changed 2 years ago by neaf

Add test for invalid argument (not JSON serializable)

comment:11 Changed 2 years ago by cypreess

  • Triage Stage changed from Accepted to Ready for checkin

comment:12 Changed 2 years ago by aaugustin

  • Triage Stage changed from Ready for checkin to Accepted

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.

comment:13 Changed 2 years ago by neaf

  • Version changed from 1.4-alpha-1 to SVN

comment:14 Changed 2 years ago by aaugustin

  • Easy pickings unset
  • Triage Stage changed from Accepted to Design decision needed

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?

comment:15 Changed 2 years ago 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": "&lt;/script&gt;&lt;script&gt;Attack!&lt;/script&gt;&lt;script&gt;"}, "e\&quot;eer", "&lt;/script&gt;&lt;script&gt;More attack!&lt;/script&gt;&lt;script&gt;"]

This works because iterencode() returns string-based dictionary keys and values as "<string>", and strings in lists as , "<string>" .

comment:16 Changed 2 years ago by aaugustin

  • Triage Stage changed from Design decision needed to Accepted

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.

comment:17 Changed 2 years ago 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'"&lt;&gt;"'
>>> 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.

Changed 2 years ago by aaugustin

comment:18 Changed 2 years ago by aaugustin

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

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 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 :)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.