From 4f79a8042e0ee73fa6afb48a992c9c07d286d699 Mon Sep 17 00:00:00 2001
From: Patrick Steele-Idem <pnidem@gmail.com>
Date: Fri, 1 Jul 2016 10:54:11 -0600
Subject: [PATCH] Fixes #322 - Autoescaping doesen't work in <script> tag
 context

---
 compiler/CodeGenerator.js                     |  2 +-
 compiler/CompileContext.js                    | 20 +++++++++++++++++-
 compiler/ast/HtmlElement.js                   | 15 +++++++++++++
 compiler/ast/Text.js                          |  8 ++++++-
 runtime/helpers.js                            | 21 +++++++++++++++++++
 .../render/escape-script/expected.html        |  3 +++
 .../render/escape-script/template.marko       |  4 ++++
 test/autotests/render/escape-script/test.js   |  5 +++++
 .../render/script-tag-entities/expected.html  |  2 +-
 .../render/script-tag-entities/test.js        |  2 +-
 10 files changed, 77 insertions(+), 5 deletions(-)
 create mode 100644 test/autotests/render/escape-script/expected.html
 create mode 100644 test/autotests/render/escape-script/template.marko
 create mode 100644 test/autotests/render/escape-script/test.js

diff --git a/compiler/CodeGenerator.js b/compiler/CodeGenerator.js
index 42a20b21e9..c2e437cabf 100644
--- a/compiler/CodeGenerator.js
+++ b/compiler/CodeGenerator.js
@@ -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);
         }
 
diff --git a/compiler/CompileContext.js b/compiler/CompileContext.js
index c80b85993d..46398aae23 100644
--- a/compiler/CompileContext.js
+++ b/compiler/CompileContext.js
@@ -80,7 +80,7 @@ class CompileContext {
     }
 
     setFlag(name) {
-        this._flags[name] = true;
+        this.pushFlag(name);
     }
 
     clearFlag(name) {
@@ -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];
diff --git a/compiler/ast/HtmlElement.js b/compiler/ast/HtmlElement.js
index 2a413d7794..1977a849e1 100644
--- a/compiler/ast/HtmlElement.js
+++ b/compiler/ast/HtmlElement.js
@@ -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');
@@ -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) {
diff --git a/compiler/ast/Text.js b/compiler/ast/Text.js
index d719dce848..1e831c1251 100644
--- a/compiler/ast/Text.js
+++ b/compiler/ast/Text.js
@@ -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 ]);
diff --git a/runtime/helpers.js b/runtime/helpers.js
index a5ce6fc2ac..510a25e543 100644
--- a/runtime/helpers.js
+++ b/runtime/helpers.js
@@ -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) {
@@ -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
diff --git a/test/autotests/render/escape-script/expected.html b/test/autotests/render/escape-script/expected.html
new file mode 100644
index 0000000000..0296a3c50a
--- /dev/null
+++ b/test/autotests/render/escape-script/expected.html
@@ -0,0 +1,3 @@
+<script>
+    var foo = {"name":"Evil \u003C/script>"};
+</script><pre>{"name":"Evil &lt;/script>"}</pre>
\ No newline at end of file
diff --git a/test/autotests/render/escape-script/template.marko b/test/autotests/render/escape-script/template.marko
new file mode 100644
index 0000000000..f12f01aad7
--- /dev/null
+++ b/test/autotests/render/escape-script/template.marko
@@ -0,0 +1,4 @@
+<script>
+    var foo = ${JSON.stringify(data.foo)};
+</script>
+<pre>${JSON.stringify(data.foo)}</pre>
\ No newline at end of file
diff --git a/test/autotests/render/escape-script/test.js b/test/autotests/render/escape-script/test.js
new file mode 100644
index 0000000000..e0035678a6
--- /dev/null
+++ b/test/autotests/render/escape-script/test.js
@@ -0,0 +1,5 @@
+exports.templateData = {
+    foo: {
+        name: 'Evil </script>'
+    }
+};
diff --git a/test/autotests/render/script-tag-entities/expected.html b/test/autotests/render/script-tag-entities/expected.html
index 91a5cf44b4..b0ca3c7a9c 100644
--- a/test/autotests/render/script-tag-entities/expected.html
+++ b/test/autotests/render/script-tag-entities/expected.html
@@ -1 +1 @@
-<script>document.write('<div>Hello &lt;script>evil&lt;script></div>');</script>
\ No newline at end of file
+<script>document.write('<div>Hello <script>evil\u003C/script></div>');</script>
\ No newline at end of file
diff --git a/test/autotests/render/script-tag-entities/test.js b/test/autotests/render/script-tag-entities/test.js
index d7f36b92c1..4221021f44 100644
--- a/test/autotests/render/script-tag-entities/test.js
+++ b/test/autotests/render/script-tag-entities/test.js
@@ -1,3 +1,3 @@
 exports.templateData = {
-    "name": "<script>evil<script>"
+    "name": "<script>evil</script>"
 };