From a4f5fd8104eee507622ebb235c8f7df81ee22dd4 Mon Sep 17 00:00:00 2001 From: Tiago Azevedo Date: Thu, 10 Sep 2015 15:33:36 +0100 Subject: [PATCH] Stringify whole app state, as keys are also vulnerable to XSS injection --- lib/application.js | 7 ++- lib/server/render.js | 105 +++---------------------------------------- spec/server_spec.ls | 5 +-- src/application.ls | 8 ++-- src/server/render.ls | 9 +--- 5 files changed, 19 insertions(+), 115 deletions(-) diff --git a/lib/application.js b/lib/application.js index 0872ce1..c62a635 100644 --- a/lib/application.js +++ b/lib/application.js @@ -1,5 +1,5 @@ (function(){ - var bluebird, cursor, dom, routes, serverRendering, unescape, domUtils, span, appComponent, initAppState, observePageChange; + var bluebird, cursor, dom, routes, serverRendering, unescape, domUtils, unesc, span, appComponent, initAppState, observePageChange; bluebird = require('bluebird'); cursor = require('./cursor'); dom = require('./dom'); @@ -7,6 +7,7 @@ serverRendering = require('./server-rendering'); unescape = require('lodash/string/unescape'); domUtils = require('./virtual-dom-utils'); + unesc = require('lodash/string/unescape'); span = dom.span; appComponent = React.createFactory(React.createClass({ displayName: 'arch-application', @@ -61,7 +62,9 @@ path = location.pathname + location.search + location.hash; rootDomNode = document.getElementById("application"); stateNode = document.getElementById("arch-state"); - serverState = JSON.parse(unescape(stateNode.text)); + serverState = JSON.parse( + unesc( + stateNode.text)); appState = serverState ? cursor(serverState) : initAppState(app.getInitialState(), routes.resolve(routeSet, path)); diff --git a/lib/server/render.js b/lib/server/render.js index 68ebb66..664200c 100644 --- a/lib/server/render.js +++ b/lib/server/render.js @@ -1,27 +1,18 @@ (function(){ - var xssFilters, jade, path, prelude, map, __template, escapeFilter; + var xssFilters, jade, path, prelude, map, __template; xssFilters = require('xss-filters'); jade = require('jade'); path = require('path'); prelude = require('prelude-ls'); map = prelude.Obj.map; __template = jade.compileFile(path.join(__dirname, 'index.jade')); - exports.escapeState = function(it){ - return xssFilters.inHTMLData(it); - }; - escapeFilter = function(k, v){ - if (deepEq$(typeof v, 'string', '===')) { - return exports.escapeState(v); - } else { - return v; - } - }; exports.stringifyState = function(it){ - return JSON.stringify(it, escapeFilter); + return xssFilters.inHTMLData( + JSON.stringify( + it)); }; exports.renderBody = function(meta, body, appState, options){ - var stringifyState, bundlePath, archBody, layout, title; - stringifyState = exports.stringifyState; + var bundlePath, archBody, layout, title; bundlePath = options.environment === 'development' ? "http://localhost:3001/app.js" : "/" + options.paths['public'] + "/app.js"; @@ -29,7 +20,7 @@ 'public': options.paths['public'], bundle: bundlePath, body: body, - state: stringifyState(appState) + state: exports.stringifyState(appState) }); layout = meta.layout, title = meta.title; return layout({ @@ -37,88 +28,4 @@ title: title }); }; - function deepEq$(x, y, type){ - var toString = {}.toString, hasOwnProperty = {}.hasOwnProperty, - has = function (obj, key) { return hasOwnProperty.call(obj, key); }; - var first = true; - return eq(x, y, []); - function eq(a, b, stack) { - var className, length, size, result, alength, blength, r, key, ref, sizeB; - if (a == null || b == null) { return a === b; } - if (a.__placeholder__ || b.__placeholder__) { return true; } - if (a === b) { return a !== 0 || 1 / a == 1 / b; } - className = toString.call(a); - if (toString.call(b) != className) { return false; } - switch (className) { - case '[object String]': return a == String(b); - case '[object Number]': - return a != +a ? b != +b : (a == 0 ? 1 / a == 1 / b : a == +b); - case '[object Date]': - case '[object Boolean]': - return +a == +b; - case '[object RegExp]': - return a.source == b.source && - a.global == b.global && - a.multiline == b.multiline && - a.ignoreCase == b.ignoreCase; - } - if (typeof a != 'object' || typeof b != 'object') { return false; } - length = stack.length; - while (length--) { if (stack[length] == a) { return true; } } - stack.push(a); - size = 0; - result = true; - if (className == '[object Array]') { - alength = a.length; - blength = b.length; - if (first) { - switch (type) { - case '===': result = alength === blength; break; - case '<==': result = alength <= blength; break; - case '<<=': result = alength < blength; break; - } - size = alength; - first = false; - } else { - result = alength === blength; - size = alength; - } - if (result) { - while (size--) { - if (!(result = size in a == size in b && eq(a[size], b[size], stack))){ break; } - } - } - } else { - if ('constructor' in a != 'constructor' in b || a.constructor != b.constructor) { - return false; - } - for (key in a) { - if (has(a, key)) { - size++; - if (!(result = has(b, key) && eq(a[key], b[key], stack))) { break; } - } - } - if (result) { - sizeB = 0; - for (key in b) { - if (has(b, key)) { ++sizeB; } - } - if (first) { - if (type === '<<=') { - result = size < sizeB; - } else if (type === '<==') { - result = size <= sizeB - } else { - result = size === sizeB; - } - } else { - first = false; - result = size === sizeB; - } - } - } - stack.pop(); - return result; - } - } }).call(this); diff --git a/spec/server_spec.ls b/spec/server_spec.ls index abf96d1..f1fc955 100644 --- a/spec/server_spec.ls +++ b/spec/server_spec.ls @@ -56,10 +56,8 @@ describe "server" (_) -> title: 'Hello' spy-on render, 'stringifyState' .and.call-through! - spy-on render, 'escapeState' .and.call-through! output = render.render-body meta, 'body', { unsafe: '' }, paths: { layouts: support-templates, public: 'dist' } expect render.stringify-state .to-have-been-called! - expect render.escape-state .to-have-been-called! it "stringifies state to JSON" -> state = { a: true, b: false }; @@ -68,7 +66,7 @@ describe "server" (_) -> it "escapes injected script tags" -> state = '' - expect (render.escape-state state) .not.to-match /^\<\/script>/ + expect (render.stringify-state state) .not.to-match /^\<\/script>/ describe "form processing" (_) -> it "passes through to application's form-processing" @@ -76,4 +74,3 @@ describe "server" (_) -> it "renders into a provided layout" it "handles a redirect as a 302" - diff --git a/src/application.ls b/src/application.ls index ac4b1c8..6c7d6c8 100644 --- a/src/application.ls +++ b/src/application.ls @@ -1,5 +1,8 @@ require! <[ bluebird ./cursor ./dom ./routes ./server-rendering lodash/string/unescape ]> -require! './virtual-dom-utils': 'dom-utils' +require! { + './virtual-dom-utils': 'dom-utils' + 'lodash/string/unescape': 'unesc' +} {span} = dom @@ -55,7 +58,7 @@ module.exports = # Initialise app state state-node = document.get-element-by-id "arch-state" - server-state = JSON.parse unescape state-node.text + server-state = state-node.text |> unesc |> JSON.parse app-state = if server-state cursor server-state @@ -121,4 +124,3 @@ module.exports = body = unless location then React.render-to-string root-element else null [meta, app-state.deref!, body, location] - diff --git a/src/server/render.ls b/src/server/render.ls index 1d25fa4..1d8627e 100644 --- a/src/server/render.ls +++ b/src/server/render.ls @@ -5,16 +5,11 @@ map = prelude.Obj.map; __template = jade.compile-file (path.join __dirname, 'index.jade') -exports.escape-state = -> xss-filters.inHTMLData it; - -escape-filter = (k, v) -> if typeof v === 'string' then return exports.escape-state v else return v - -exports.stringify-state = -> JSON.stringify it, escape-filter +exports.stringify-state = -> it |> JSON.stringify |> xss-filters.inHTMLData exports.render-body = (meta, body, app-state, options) -> - stringify-state = exports.stringify-state bundle-path = if options.environment is 'development' then "http://localhost:3001/app.js" else "/#{options.paths.public}/app.js" - arch-body = __template public: options.paths.public, bundle: bundle-path, body: body, state: stringify-state app-state + arch-body = __template public: options.paths.public, bundle: bundle-path, body: body, state: exports.stringify-state app-state {layout, title} = meta layout body: arch-body, title: title