git commit: [#6604] escape < to \u003C in JSON to ensure it cannot be parsed as HTML and be subject to XSS

[jo...@apache.org]

Updated Branches:
  refs/heads/master e42b20b37 -> 1843655af


[#6604] escape < to \u003C in JSON to ensure it cannot be parsed as HTML and be subject to XSS


Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/1843655a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/1843655a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/1843655a

Branch: refs/heads/master
Commit: 1843655afd93053119ad454a6628083c0e06284d
Parents: e42b20b
Author: Dave Brondsema <db...@slashdotmedia.com>
Authored: Mon Aug 26 16:35:51 2013 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Mon Aug 26 16:53:30 2013 +0000

----------------------------------------------------------------------
 Allura/allura/lib/patches.py                    | 23 ++++++++++++++++++++
 .../forgewiki/tests/functional/test_rest.py     | 21 +++++++++++++++++-
 2 files changed, 43 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/1843655a/Allura/allura/lib/patches.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/patches.py b/Allura/allura/lib/patches.py
index 60c4648..804c947 100644
--- a/Allura/allura/lib/patches.py
+++ b/Allura/allura/lib/patches.py
@@ -15,10 +15,14 @@
 #       specific language governing permissions and limitations
 #       under the License.
 
+import re
+
 import webob
 import tg.decorators
 from decorator import decorator
 from pylons import request
+import mock
+import simplejson
 
 from allura.lib import helpers as h
 
@@ -78,6 +82,25 @@ def apply():
         return func(*args, **kwargs)
 
 
+    # http://blog.watchfire.com/wfblog/2011/10/json-based-xss-exploitation.html
+    # change < to its unicode escape when rendering JSON out of turbogears
+    # This is to avoid IE9 and earlier, which don't know the json content type
+    # and may attempt to render JSON data as HTML if the URL ends in .html
+    
+    original_tg_jsonify_GenericJSON_encode = tg.jsonify.GenericJSON.encode
+    escape_pattern_with_lt = re.compile(simplejson.encoder.ESCAPE.pattern.rstrip(']') + '<' + ']')
+
+    @h.monkeypatch(tg.jsonify.GenericJSON)
+    def encode(self, o):
+        # ensure_ascii=False forces encode_basestring() to be called instead of
+        # encode_basestring_ascii() and encode_basestring_ascii may likely be c-compiled
+        # and thus not monkeypatchable
+        with h.push_config(self, ensure_ascii=False), \
+             h.push_config(simplejson.encoder, ESCAPE=escape_pattern_with_lt), \
+             mock.patch.dict(simplejson.encoder.ESCAPE_DCT, {'<': r'\u003C'}):
+            return original_tg_jsonify_GenericJSON_encode(self, o)
+
+
 # must be saved outside the newrelic() method so that multiple newrelic()
 # calls (e.g. during tests) don't cause the patching to get applied to itself
 # over and over

http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/1843655a/ForgeWiki/forgewiki/tests/functional/test_rest.py
----------------------------------------------------------------------
diff --git a/ForgeWiki/forgewiki/tests/functional/test_rest.py b/ForgeWiki/forgewiki/tests/functional/test_rest.py
index 0025150..0b93680 100644
--- a/ForgeWiki/forgewiki/tests/functional/test_rest.py
+++ b/ForgeWiki/forgewiki/tests/functional/test_rest.py
@@ -19,7 +19,9 @@
 
 import json
 
-from nose.tools import assert_equal
+from nose.tools import assert_equal, assert_in
+import simplejson
+import tg
 
 from allura.lib import helpers as h
 from allura.tests import decorators as td
@@ -76,3 +78,20 @@ class TestWikiApi(TestRestApiBase):
         r = self.api_get(u'/rest/p/test/wiki/tést/'.encode('utf-8'))
         assert_equal(r.json['text'], data['text'])
         assert_equal(r.json['labels'], data['labels'].split(','))
+
+    # http://blog.watchfire.com/wfblog/2011/10/json-based-xss-exploitation.html
+    def test_json_encoding_security(self):
+        self.api_post('/rest/p/test/wiki/foo.html',
+                      text='foo <img src=x onerror=alert(1)> bar')
+        r = self.api_get('/rest/p/test/wiki/foo.html')
+        # raw text is not an HTML tag
+        assert_in(r'foo \u003Cimg src=x onerror=alert(1)> bar', r.body)
+        # and json still is parsed into correct content
+        assert_equal(r.json['text'], 'foo <img src=x onerror=alert(1)> bar')
+
+    def test_json_encoding_directly(self):
+        # used in @expose('json')
+        assert_equal(tg.jsonify.encode('<'), '"\u003C"')
+        # make sure these are unchanged
+        assert_equal(json.dumps('<'), '"<"')
+        assert_equal(simplejson.dumps('<'), '"<"')