From 430f88097233695f6bc0cdfa6f594ad126930345 Mon Sep 17 00:00:00 2001
From: Dervus Grim <dervus@lavabit.com>
Date: Fri, 26 Apr 2013 15:47:55 +0200
Subject: [PATCH] Close security issue in !!js/function constructor.

Prevent it from code execution on parsing.
Thanks to @nealpoole for report.
---
 HISTORY.md                                    |  7 +++++
 lib/js-yaml/type/js/function.js               | 29 ++++++++++++++++---
 package.json                                  |  3 +-
 test/issues.js                                |  1 +
 .../data/issue-parse-function-security.yml    |  3 ++
 test/issues/issue-parse-function-security.js  | 22 ++++++++++++++
 6 files changed, 60 insertions(+), 5 deletions(-)
 create mode 100644 test/issues/data/issue-parse-function-security.yml
 create mode 100644 test/issues/issue-parse-function-security.js

diff --git a/HISTORY.md b/HISTORY.md
index 71205e96..5908a9bc 100644
--- a/HISTORY.md
+++ b/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
 ------------------
 
diff --git a/lib/js-yaml/type/js/function.js b/lib/js-yaml/type/js/function.js
index 6bc05c45..4b3b3ca5 100644
--- a/lib/js-yaml/type/js/function.js
+++ b/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;
   }
 }
diff --git a/package.json b/package.json
index d3762db4..936e2206 100644
--- a/package.json
+++ b/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" }
 }
diff --git a/test/issues.js b/test/issues.js
index 94d7a54d..11ffd183 100644
--- a/test/issues.js
+++ b/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');
 });
diff --git a/test/issues/data/issue-parse-function-security.yml b/test/issues/data/issue-parse-function-security.yml
new file mode 100644
index 00000000..812e9c9c
--- /dev/null
+++ b/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)'
diff --git a/test/issues/issue-parse-function-security.js b/test/issues/issue-parse-function-security.js
new file mode 100644
index 00000000..f767959a
--- /dev/null
+++ b/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, []);
+});