Merge pull request #55 from neraliu/strict-context-parser-integration

strict context parser and strict css parser integration

Author: neraliu

Gruntfile.js

@@ -10,7 +10,7 @@ module.exports = function(grunt) {
     execute: {
       cssparser: {
         options: {
-          args: ['src/css-parser/css-parser.21.attr.partial.y', 'src/css-parser/css.21.l', '--outfile', 'src/css-parser/css-parser.js']
+          args: ['src/css-parser/css-parser.strict.attr.partial.y', 'src/css-parser/css.strict.l', '--outfile', 'src/css-parser/css-parser.js']
         },
         src: ['node_modules/jison/lib/cli.js']
       },
@@ -81,6 +81,9 @@ module.exports = function(grunt) {
         options: {
           excludes: [
             'src/css-parser/css-parser.js', 
+            'src/html-decoder/*.js',
+            'src/html-decoder/gen/*.js',
+            'src/html-decoder/polyfills/*.js',
             'src/polyfills/browser.js',
             'src/polyfills/minimal.js'
           ],

package.json

@@ -1,6 +1,6 @@
 {
   "name": "secure-handlebars",
-  "version": "0.3.0",
+  "version": "1.0.0",
   "licenses": [
     {
       "type": "BSD",
@@ -41,9 +41,8 @@
     "test": "grunt test"
   },
   "dependencies": {
-    "context-parser": "^1.0.0",
-    "debug": "^2.1.2",
-    "xss-filters": "^1.1.0"
+    "context-parser": "^1.1.0",
+    "xss-filters": "^1.2.0"
   },
   "devDependencies": {
     "bluebird": "^2.9.21",
@@ -56,8 +55,9 @@
     "grunt-contrib-uglify": "^0.9.1",
     "grunt-execute": "^0.2.2",
     "grunt-karma": "^0.11.0",
-    "karma-mocha": "~0.1.10",
     "grunt-mocha-istanbul": "^2.3.0",
+    "karma": "^0.12.36",
+    "karma-mocha": "~0.1.10",
     "karma-phantomjs-launcher": "^0.2.0",
     "handlebars": "^3.0.1"
   },

src/context-parser-handlebars.js

@@ -11,11 +11,14 @@ Authors: Nera Liu <neraliu@yahoo-inc.com>
 (function () {
 "use strict";
 
-/* debug facility */
-var debug = require('debug')('cph');
-
 /* import the required package */
 var ContextParser = require('./strict-context-parser.js'),
+    configContextParser = {
+        enableInputPreProcessing: true,
+        enableCanonicalization: true,
+        enableIEConditionalComments: true,
+        enableStateTracking: true
+    },
     handlebarsUtils = require('./handlebars-utils.js'),
     stateMachine = require('context-parser').StateMachine;
 
@@ -107,9 +110,8 @@ function ContextParserHandlebars(config) {
     this._charNo = 0;
     this._lineNo = 1;
 
-    // TODO: enforce the strict CP by overridding the config object.
     /* context parser for HTML5 parsing */
-    this.contextParser = new ContextParser(config);
+    this.contextParser = new ContextParser(configContextParser);
 }
 
 /**
@@ -400,7 +402,7 @@ ContextParserHandlebars.prototype.generateNodeObject = function(type, content, s
 ContextParserHandlebars.prototype.analyzeAst = function(ast, contextParser, charNo) {
 
     var output = '', leftParser, rightParser,
-        t, msg, exceptionObj, debugString = [];
+        t, msg, exceptionObj;
 
     this._charNo = charNo;
 
@@ -412,7 +414,7 @@ ContextParserHandlebars.prototype.analyzeAst = function(ast, contextParser, char
 
             if (node.type === 'html') {
 
-                output += parser.parsePartial(node.content);
+                output += parser.contextualize(node.content);
 
             } else if (node.type === 'escapeexpression' ||
                 node.type === 'rawexpression') {
@@ -426,10 +428,7 @@ ContextParserHandlebars.prototype.analyzeAst = function(ast, contextParser, char
             } else if (node.type === 'node') {
 
                 t = this.analyzeAst(node.content, parser, node.startPos);
-                // cloning states from the branches
-                parser.state = t.parser.state;
-                parser.attributeName = t.parser.attributeName;
-                parser.attributeValue = t.parser.attributeValue;
+                parser.cloneStates(t.parser);
 
                 output += t.output;
 
@@ -467,7 +466,6 @@ ContextParserHandlebars.prototype.analyzeAst = function(ast, contextParser, char
              leftParser.getAttributeNameType() !== rightParser.getAttributeNameType())
             )
             ) {
-        // debug("analyzeAst:["+r.parsers[0].state+"/"+r.parsers[1].state+"]");
         msg = "[ERROR] SecureHandlebars: Inconsistent HTML5 state after conditional branches. Please fix your template! ";
         msg += "state:("+leftParser.state+"/"+rightParser.state+"),";
         msg += "attributeNameType:("+leftParser.getAttributeNameType()+"/"+rightParser.getAttributeNameType()+")";
@@ -510,7 +508,6 @@ ContextParserHandlebars.prototype.handleTemplate = function(input, i, stateObj)
         if (input[i] === '{' && i+2<len && input[i+1] === '{' && input[i+2] === '{') {
             handlebarsExpressionType = handlebarsUtils.RAW_EXPRESSION;
             /* _handleRawExpression and no validation need, it is safe guard in buildAst function */
-            debug("handleTemplate:handlebarsExpressionType:"+handlebarsExpressionType,",i:"+i+",state:"+stateObj.state);
             obj = this.consumeExpression(input, i, handlebarsExpressionType, true);
             return;
         } else if (input[i] === '{' && i+1<len && input[i+1] === '{') {
@@ -519,7 +516,6 @@ ContextParserHandlebars.prototype.handleTemplate = function(input, i, stateObj)
             switch (handlebarsExpressionType) {
                 case handlebarsUtils.ESCAPE_EXPRESSION:
                     /* handleEscapeExpression and no validation need, it is safe guard in buildAst function */
-                    debug("handleTemplate:handlebarsExpressionType:"+handlebarsExpressionType,",i:"+i+",state:"+stateObj.state);
                     obj = this.handleEscapeExpression(input, i, len, stateObj, true);
                     return;
                 default:
@@ -553,8 +549,8 @@ ContextParserHandlebars.prototype.addFilters = function(parser, input) {
     var isFullUri = false, filters = [], f, exceptionObj, errorMessage,
         state = parser.state,
         tagName = parser.getStartTagName(),
-        attributeName = parser.attributeName,
-        attributeValue = parser.attributeValue;
+        attributeName = parser.getAttributeName(),
+        attributeValue = parser.getAttributeValue();
 
     try {
 
@@ -594,9 +590,13 @@ ContextParserHandlebars.prototype.addFilters = function(parser, input) {
                     filters.push(f);                    
 
                 } else if (parser.getAttributeNameType() === ContextParser.ATTRTYPE_CSS) { // CSS
-
-                    attributeValue = htmlDecoder.decode(attributeValue);
-                    var r = cssParserUtils.parseStyleAttributeValue(attributeValue);
+                    var r;
+                    try {
+                        attributeValue = htmlDecoder.decode(attributeValue);
+                        r = cssParserUtils.parseStyleAttributeValue(attributeValue);
+                    } catch (e) {
+                        throw 'Unsafe output expression @ attribute style CSS context (Parsing error OR expression position not supported!)';
+                    }
                     switch(r.code) {
                         case cssParserUtils.STYLE_ATTRIBUTE_URL_UNQUOTED:
                             filters.push(filter.FILTER_ATTRIBUTE_VALUE_STYLE_EXPR_URL_UNQUOTED);
@@ -694,11 +694,10 @@ ContextParserHandlebars.prototype.addFilters = function(parser, input) {
             // TODO: need tagname tracing in Context Parser such that we can have 
             // ability to capture the case of putting output expression within dangerous tag.
             // like svg etc.
-            // the following will be caught by parser.isScriptableTag() anyway
-            // case stateMachine.State.STATE_SCRIPT_DATA: // 6
-            //     throw 'inside <script> tag (i.e., SCRIPT_DATA state)';
+            // the following will be caught by handlebarsUtils.isScriptableTag(tagName) anyway
+            case stateMachine.State.STATE_SCRIPT_DATA: // 6
+                throw 'inside <script> tag (i.e., SCRIPT_DATA state)';
 
-
             // should not fall into the following states
             case stateMachine.State.STATE_BEFORE_ATTRIBUTE_VALUE: // 37
                 throw 'unexpectedly BEFORE_ATTRIBUTE_VALUE state';
@@ -715,7 +714,7 @@ ContextParserHandlebars.prototype.addFilters = function(parser, input) {
             // To be secure, scriptable tags when encountered will anyway throw an error/warning
             // they require either special parsers of their own context (e.g., CSS/script parsers) 
             //    or an application-specific whitelisted url check (e.g., <script src=""> with yubl-yavu-yufull is not enough)
-            errorMessage += parser.isScriptableTag() ? 'scriptable <' + tagName + '> tag' : exception;
+            errorMessage += handlebarsUtils.isScriptableTag(tagName) ? 'scriptable <' + tagName + '> tag' : exception;
 
             exceptionObj = new ContextParserHandlebarsException(errorMessage, this._lineNo, this._charNo);
             handlebarsUtils.handleError(exceptionObj, this._config._strictMode);

…/css-parser/css-parser.21.attr.partial.y …-parser/css-parser.strict.attr.partial.ysrc/css-parser/css-parser.21.attr.partial.y renamed to src/css-parser/css-parser.strict.attr.partial.y src/css-parser/css-parser.21.attr.partial.y renamed to src/css-parser/css-parser.strict.attr.partial.y

@@ -5,8 +5,6 @@ See the accompanying LICENSE file for terms.
 
 Authors: Nera Liu <neraliu@yahoo-inc.com, neraliu@gmail.com>
 
-Grammer of CSS 2.1 specification
-
 Reference:
 - http://www.w3.org/TR/2011/REC-CSS2-20110607/syndata.html
 - http://www.w3.org/TR/2011/REC-CSS2-20110607/grammar.html