Close security issue in !!js/function constructor.

Prevent it from code execution on parsing.
Thanks to @nealpoole for report.

Author: Dervus Grim

HISTORY.md

@@ -1,3 +1,10 @@
+2.0.5 / --
+------------------
+
+* Close security issue in !!js/function constructor.
+  Big thanks to @nealpoole for security audit.
+
+
 2.0.4 / 2013-04-08
 ------------------
 

lib/js-yaml/type/js/function.js

@@ -1,18 +1,39 @@
 'use strict';
 
 
+var esprima = require('esprima');
+
+
 var NIL  = require('../../common').NIL;
 var Type = require('../../type');
 
 
 function resolveJavascriptFunction(object /*, explicit*/) {
   /*jslint evil:true*/
-  var func;
 
   try {
-    func = new Function('return ' + object);
-    return func();
-  } catch (error) {
+    var source = '(' + object + ')',
+        ast    = esprima.parse(source, { range: true }),
+        params = [],
+        body;
+
+    if ('Program'             !== ast.type         ||
+        1                     !== ast.body.length  ||
+        'ExpressionStatement' !== ast.body[0].type ||
+        'FunctionExpression'  !== ast.body[0].expression.type) {
+      return NIL;
+    }
+
+    ast.body[0].expression.params.forEach(function (param) {
+      params.push(param.name);
+    });
+
+    body = ast.body[0].expression.body.range;
+
+    // Esprima's ranges include the first '{' and the last '}' characters on
+    // function expressions. So cut them out.
+    return new Function(params, source.slice(body[0]+1, body[1]-1));
+  } catch (err) {
     return NIL;
   }
 }

package.json

@@ -32,7 +32,8 @@
   "bin"             : { "js-yaml": "bin/js-yaml.js" },
   "scripts"         : { "test": "make test" },
 
-  "dependencies"    : { "argparse": "~ 0.1.11" },
+  "dependencies"    : { "argparse": "~ 0.1.11",
+                        "esprima":  "~ 1.0.2" },
   "devDependencies" : { "mocha": "*" },
   "engines"         : { "node": ">= 0.6.0" }
 }

test/issues.js

@@ -11,4 +11,5 @@ describe('Issues.', function () {
   require('./issues/issue-46.js');
   require('./issues/issue-54.js');
   require('./issues/issue-64.js');
+  require('./issues/issue-parse-function-security.js');
 });

test/issues/data/issue-parse-function-security.yml

@@ -0,0 +1,3 @@
+tests:
+  - !!js/function 'makeBadThing("BAD THING 1")'
+  - !!js/function 'function () { makeBadThing("BAD THING 2") }.call(this)'

test/issues/issue-parse-function-security.js

@@ -0,0 +1,22 @@
+'use strict';
+/*global it */
+
+
+var assert = require('assert');
+
+
+var badThings = [];
+
+
+global.makeBadThing = function (thing) {
+  badThings.push(thing);
+};
+
+
+it('Function constructor must not allow to execute any code while parsing.', function () {
+  assert.throws(function () {
+    require('./data/issue-parse-function-security.yml');
+  });
+
+  assert.deepEqual(badThings, []);
+});