From fe12fb4a1349dc74c958538aa78e474b6df32f75 Mon Sep 17 00:00:00 2001 From: ccd0 Date: Thu, 14 Apr 2016 17:56:56 -0700 Subject: [PATCH] Validate and parenthesize JS in HTML templates so we don't accidentally break out of placeholders. --- Makefile | 2 +- package.json | 1 + tools/template.js | 22 ++++++++++++++++------ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 3d305d812..e7c979b6d 100644 --- a/Makefile +++ b/Makefile @@ -19,7 +19,7 @@ endif coffee := $(BIN)coffee -c --no-header coffee_deps := node_modules/coffee-script/package.json template := node tools/template.js -template_deps := package.json tools/template.js node_modules/lodash/package.json +template_deps := package.json tools/template.js node_modules/lodash/package.json node_modules/esprima/package.json cat := node tools/cat.js cat_deps := tools/cat.js diff --git a/package.json b/package.json index f1d850062..50e6522a5 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "devDependencies": { "coffee-script": "1.9.3", "crx": "^3.0.3", + "esprima": "^2.7.2", "font-awesome": "4.5.0", "grunt": "^1.0.1", "grunt-shell": "^1.2.1", diff --git a/tools/template.js b/tools/template.js index 587f9f14c..76047a239 100644 --- a/tools/template.js +++ b/tools/template.js @@ -1,5 +1,6 @@ var fs = require('fs'); var _ = require('lodash'); +var esprima = require('esprima'); // disable ES6 delimiters _.templateSettings.interpolate = /<%=([\s\S]+?)%>/g; @@ -138,14 +139,23 @@ Placeholder.prototype.allowed = function(context) { }; Placeholder.prototype.build = function() { - // first argument is always JS expression; add backticks for embedding it in Coffeescript - var expr = '`'+this.args[0]+'`'; + // first argument is always JS expression; validate it so we don't accidentally break out of placeholders + var expr = this.args[0]; + var ast; + try { + ast = esprima.parse(expr); + } catch (err) { + throw new Error(`Invalid JavaScript in template (${expr})`); + } + if (!(ast.type === 'Program' && ast.body.length == 1 && ast.body[0].type === 'ExpressionStatement')) { + throw new Error(`JavaScript in template is not an expression (${expr})`); + } switch(this.type) { - case '$': return `E(${expr})`; // $ : escaped text - case '&': return `${expr}.innerHTML`; // & : contents of HTML element or template (of form {innerHTML: "safeHTML"}) - case '@': return `E.cat(${expr})`; // @ : contents of array of HTML elements or templates (see src/General/Globals.coffee for E.cat) + case '$': return `\`E(${expr})\``; // $ : escaped text + case '&': return `\`(${expr}).innerHTML\``; // & : contents of HTML element or template (of form {innerHTML: "safeHTML"}) + case '@': return `\`E.cat(${expr})\``; // @ : contents of array of HTML elements or templates (see src/General/Globals.coffee for E.cat) case '?': - return `(if ${expr} then ${this.args[1] || '""'} else ${this.args[2] || '""'})`; // ? : conditional expression + return `(if \`(${expr})\` then ${this.args[1] || '""'} else ${this.args[2] || '""'})`; // ? : conditional expression } throw new Error(`Unrecognized placeholder type (${this.type})`); };