Patchwork [3,of,3] encoding: backport paranoid escaping from templatefilters.jsonescape()

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 16, 2016, 12:33 p.m.
Message ID <a090fb7be0b0415289ca.1452947635@mimosa>
Download mbox | patch
Permalink /patch/12793/
State Superseded
Commit b2d24c2898f91c891425c204d4f8e683d8120372
Headers show

Comments

Yuya Nishihara - Jan. 16, 2016, 12:33 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1451213891 -32400
#      Sun Dec 27 19:58:11 2015 +0900
# Node ID a090fb7be0b0415289ca2b6666c5f0cd8d912496
# Parent  b3b1bef76d54a4755a1b221a36b00253eefefd9a
encoding: backport paranoid escaping from templatefilters.jsonescape()

This was introduced by 55c763926a28. It shouldn't be necessary, but it would
prevent possible XSS vulnerabilities.
Matt Mackall - Jan. 16, 2016, 5:52 p.m.
On Sat, 2016-01-16 at 21:33 +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1451213891 -32400
> #      Sun Dec 27 19:58:11 2015 +0900
> # Node ID a090fb7be0b0415289ca2b6666c5f0cd8d912496
> # Parent  b3b1bef76d54a4755a1b221a36b00253eefefd9a
> encoding: backport paranoid escaping from templatefilters.jsonescape()
> 
> This was introduced by 55c763926a28. It shouldn't be necessary, but it would
> prevent possible XSS vulnerabilities.

Ok, I rechecked this. For JSON included by a template directly in HTML page
source, it is definitely required. Convince yourself here:

http://escape.alf.nu/1

Strongly recommend people who are interested in this topic spend a few hours
working through this website.

-- 
Mathematics is the supreme nostalgia of our time.
Yuya Nishihara - Jan. 17, 2016, 5 a.m.
On Sat, 16 Jan 2016 11:52:02 -0600, Matt Mackall wrote:
> On Sat, 2016-01-16 at 21:33 +0900, Yuya Nishihara wrote:
> > # HG changeset patch
> > # User Yuya Nishihara <yuya@tcha.org>
> > # Date 1451213891 -32400
> > #      Sun Dec 27 19:58:11 2015 +0900
> > # Node ID a090fb7be0b0415289ca2b6666c5f0cd8d912496
> > # Parent  b3b1bef76d54a4755a1b221a36b00253eefefd9a
> > encoding: backport paranoid escaping from templatefilters.jsonescape()
> > 
> > This was introduced by 55c763926a28. It shouldn't be necessary, but it would
> > prevent possible XSS vulnerabilities.
> 
> Ok, I rechecked this. For JSON included by a template directly in HTML page
> source, it is definitely required. Convince yourself here:
> 
> http://escape.alf.nu/1
> 
> Strongly recommend people who are interested in this topic spend a few hours
> working through this website.

Doh, I see. </script><script>
alert('Can you fix my dumb commit message?');//
Yuya Nishihara - Jan. 17, 2016, 12:55 p.m.
On Sun, 17 Jan 2016 14:00:48 +0900, Yuya Nishihara wrote:
> On Sat, 16 Jan 2016 11:52:02 -0600, Matt Mackall wrote:
> > On Sat, 2016-01-16 at 21:33 +0900, Yuya Nishihara wrote:
> > > # HG changeset patch
> > > # User Yuya Nishihara <yuya@tcha.org>
> > > # Date 1451213891 -32400
> > > #      Sun Dec 27 19:58:11 2015 +0900
> > > # Node ID a090fb7be0b0415289ca2b6666c5f0cd8d912496
> > > # Parent  b3b1bef76d54a4755a1b221a36b00253eefefd9a
> > > encoding: backport paranoid escaping from templatefilters.jsonescape()
> > > 
> > > This was introduced by 55c763926a28. It shouldn't be necessary, but it would
> > > prevent possible XSS vulnerabilities.
> > 
> > Ok, I rechecked this. For JSON included by a template directly in HTML page
> > source, it is definitely required. Convince yourself here:
> > 
> > http://escape.alf.nu/1
> > 
> > Strongly recommend people who are interested in this topic spend a few hours
> > working through this website.
> 
> Doh, I see. </script><script>
> alert('Can you fix my dumb commit message?');//

Never mind. I noticed the initialization of jsonmaps isn't thread-safe, so I
have to rework it anyway. I'll resend the whole series next month.

Patch

diff --git a/mercurial/encoding.py b/mercurial/encoding.py
--- a/mercurial/encoding.py
+++ b/mercurial/encoding.py
@@ -408,8 +408,8 @@  def jsonescape(s, paranoid=False):
     >>> jsonescape('')
     ''
 
-    If paranoid, non-ascii characters are also escaped. This is suitable for
-    web output.
+    If paranoid, non-ascii and common troublesome characters are also escaped.
+    This is suitable for web output.
 
     >>> jsonescape('escape boundary: \\x7e \\x7f \\xc2\\x80', paranoid=True)
     'escape boundary: ~ \\\\u007f \\\\u0080'
@@ -419,6 +419,8 @@  def jsonescape(s, paranoid=False):
     'utf-8: caf\\\\u00e9'
     >>> jsonescape('non-BMP: \\xf0\\x9d\\x84\\x9e', paranoid=True)
     'non-BMP: \\\\ud834\\\\udd1e'
+    >>> jsonescape('<foo@example.org>', paranoid=True)
+    '\\\\u003cfoo@example.org\\\\u003e'
     '''
 
     if not _jsonmap:
@@ -436,6 +438,8 @@  def jsonescape(s, paranoid=False):
         _jsonmap['\f'] = '\\f'
         _jsonmap['\r'] = '\\r'
         _paranoidjsonmap.update(_jsonmap)
+        _paranoidjsonmap['<'] = '\\u003c'
+        _paranoidjsonmap['>'] = '\\u003e'
         for x in xrange(128, 256):
             c = chr(x)
             _jsonmap[c] = c