Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#7177 closed (fixed)

Expand and modify escaping of JavaScript strings in the escapejs filter

Reported by: Mike Wiacek <mjwiacek@…> Owned by: nobody
Component: Template system Version: master
Severity: Keywords: aug22sprint
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

The escapejs filter currently escapes a small subset of characters to prevent JavaScript injection. However, the resulting strings can still contain valid HTML, leading to XSS vulnerabilities. Using hex encoding as opposed to backslash escaping, effectively prevents Javascript injection and also helps prevent XSS. Attached is a small patch that modifies the _js_escapes tuple to use hex encoding on an expanded set of characters.

Attachments (5)

django_js_escapes.diff (1.7 KB) - added by Mike Wiacek <mjwiacek@…> 6 years ago.
django_js_escapes.2.diff (1.7 KB) - added by Mike Wiacek <mjwiacek@…> 6 years ago.
7177.patch (3.0 KB) - added by cgrady 6 years ago.
added tests
7177-readable.diff (2.5 KB) - added by mjwiacek 6 years ago.
Same as 7177.diff with an update to the AUTHORS files and a self generating _js_escapes tuple.
7177-updated.diff (2.7 KB) - added by ericholscher 6 years ago.
Combining the code into one function

Download all attachments as: .zip

Change History (17)

Changed 6 years ago by Mike Wiacek <mjwiacek@…>

Changed 6 years ago by Mike Wiacek <mjwiacek@…>

comment:1 Changed 6 years ago by Mike Wiacek <mjwiacek@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Version 1 didn't escape \'s first. Version 2 fixes this.

comment:2 Changed 6 years ago by jacob

  • milestone set to 1.0

comment:3 Changed 6 years ago by ericholscher

  • Triage Stage changed from Unreviewed to Accepted

Changed 6 years ago by cgrady

added tests

comment:4 Changed 6 years ago by cgrady

  • Keywords aug22sprint added

comment:5 Changed 6 years ago by mtredinnick

  • Patch needs improvement set

That big table of mappings is a little unwieldy (all the hex code mappings). Would be good if that could be generated programmatically so that we have confidence there are no typos in there. I'm just talking about the \x01 -> "\x01" pieces.

comment:6 Changed 6 years ago by ericholscher

for x in range(32):

print "%02X" % x

Should do it. I'll make a patch later tonight if need be.

comment:7 Changed 6 years ago by mjwiacek@…

Working on a shorter version now.

Changed 6 years ago by mjwiacek

Same as 7177.diff with an update to the AUTHORS files and a self generating _js_escapes tuple.

comment:8 Changed 6 years ago by mjwiacek

Just note that I renamed _js_escapes to _base_js_escapes so that when the remaining escaping rules are programmatically generated, we don't end up with duplicate entries. The end result is the same, a tuple called _js_escapes.

Changed 6 years ago by ericholscher

Combining the code into one function

comment:9 Changed 6 years ago by ericholscher

Attached a diff that combines the code into one function. Also added myself to the AUTHORS file as well, because of my work on #7416 and lots of trac help.

comment:10 Changed 6 years ago by mjwiacek

I'd argue the second to most recent version is more readable. If you choose to go with the more concise version, I'd add comments so it's clear whats happening.

comment:11 Changed 6 years ago by mtredinnick

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

(In [8577]) Fixed #7177 -- Added extra robustness to the escapejs filter so that all
invalid characters are correctly escaped. This avoids any chance to inject raw
HTML inside <script> tags. Thanks to Mike Wiacek for the patch and Collin Grady
for the tests.

comment:12 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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.