Skip to content

Commit

Permalink
Fixes #322 - Autoescaping doesen't work in <script> tag context
Browse files Browse the repository at this point in the history
  • Loading branch information
patrick-steele-idem committed Jul 1, 2016
1 parent a0af149 commit 4f79a80
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 5 deletions.
2 changes: 1 addition & 1 deletion compiler/CodeGenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ class Generator {

var beforeAfterEvent;

if (node.listenerCount('beforeGenerateCode') || node.listenerCount('beforeGenerateCode')) {
if (node.listenerCount('beforeGenerateCode') || node.listenerCount('afterGenerateCode')) {
beforeAfterEvent = new GeneratorEvent(node, this);
}

Expand Down
20 changes: 19 additions & 1 deletion compiler/CompileContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class CompileContext {
}

setFlag(name) {
this._flags[name] = true;
this.pushFlag(name);
}

clearFlag(name) {
Expand All @@ -91,6 +91,24 @@ class CompileContext {
return this._flags.hasOwnProperty(name);
}

pushFlag(name) {
if (this._flags.hasOwnProperty(name)) {
this._flags[name]++;
} else {
this._flags[name] = 1;
}
}

popFlag(name) {
if (!this._flags.hasOwnProperty(name)) {
throw new Error('popFlag() called for "' + name + '" when flag was not set');
}

if (--this._flags[name] === 0) {
delete this._flags[name];
}
}

addError(errorInfo) {
if (errorInfo instanceof Node) {
let node = arguments[0];
Expand Down
15 changes: 15 additions & 0 deletions compiler/ast/HtmlElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,18 @@ class EndTag extends Node {
}
}

function beforeGenerateCode(event) {
if (event.node.tagName === 'script') {
event.context.pushFlag('SCRIPT_BODY');
}
}

function afterGenerateCode(event) {
if (event.node.tagName === 'script') {
event.context.popFlag('SCRIPT_BODY');
}
}

class HtmlElement extends Node {
constructor(def) {
super('HtmlElement');
Expand All @@ -84,6 +96,9 @@ class HtmlElement extends Node {
this.selfClosed = def.selfClosed;
this.dynamicAttributes = undefined;
this.bodyOnlyIf = undefined;

this.on('beforeGenerateCode', beforeGenerateCode);
this.on('afterGenerateCode', afterGenerateCode);
}

generateHtmlCode(codegen) {
Expand Down
8 changes: 7 additions & 1 deletion compiler/ast/Text.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,16 @@ class Text extends Node {
let builder = codegen.builder;

if (escape) {
let escapeFuncVar = 'escapeXml';

if (codegen.context.isFlagSet('SCRIPT_BODY')) {
escapeFuncVar = codegen.addStaticVar('escapeScript', '__helpers.xs');
}

// TODO Only escape the parts that need to be escaped if it is a compound expression with static
// text parts
argument = builder.functionCall(
'escapeXml',
escapeFuncVar,
[argument]);
} else {
argument = builder.functionCall(builder.identifier('str'), [ argument ]);
Expand Down
21 changes: 21 additions & 0 deletions runtime/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var attr = require('raptor-util/attr');
var isArray = Array.isArray;
var STYLE_ATTR = 'style';
var CLASS_ATTR = 'class';
var escapeEndingScriptTagRegExp = /<\//g;

function notEmpty(o) {
if (o == null) {
Expand Down Expand Up @@ -197,6 +198,26 @@ module.exports = {
* @private
*/
xa: escapeXmlAttr,

/**
* Escapes the '</' sequence in the body of a <script> body to avoid the `<script>` being
* ended prematurely.
*
* For example:
* var evil = {
* name: '</script><script>alert(1)</script>'
* };
*
* <script>var foo = ${JSON.stringify(evil)}</script>
*
* Without escaping the ending '</script>' sequence the opening <script> tag would be
* prematurely ended and a new script tag could then be started that could then execute
* arbitrary code.
*/
xs: function(val) {
return (typeof val === 'string') ? val.replace(escapeEndingScriptTagRegExp, '\\u003C/') : val;
},

/**
* Internal method to render a single HTML attribute
* @private
Expand Down
3 changes: 3 additions & 0 deletions test/autotests/render/escape-script/expected.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<script>
var foo = {"name":"Evil \u003C/script>"};
</script><pre>{"name":"Evil &lt;/script>"}</pre>
4 changes: 4 additions & 0 deletions test/autotests/render/escape-script/template.marko
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<script>
var foo = ${JSON.stringify(data.foo)};
</script>
<pre>${JSON.stringify(data.foo)}</pre>
5 changes: 5 additions & 0 deletions test/autotests/render/escape-script/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
exports.templateData = {
foo: {
name: 'Evil </script>'
}
};
2 changes: 1 addition & 1 deletion test/autotests/render/script-tag-entities/expected.html
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<script>document.write('<div>Hello &lt;script>evil&lt;script></div>');</script>
<script>document.write('<div>Hello <script>evil\u003C/script></div>');</script>
2 changes: 1 addition & 1 deletion test/autotests/render/script-tag-entities/test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
exports.templateData = {
"name": "<script>evil<script>"
"name": "<script>evil</script>"
};

0 comments on commit 4f79a80

Please sign in to comment.