Merge pull request from GHSA-j3rv-w43q-f9x2

Fix arbitrary code execution

Author: oxyno-zeta

CHANGELOG.md

@@ -1,3 +1,7 @@
+# 2.2.2
+## Fix
+- Add `allowFunctionEvaluation` prop to mitigate a security vulnerability
+- Use [`Function`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function) instead of [`eval`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval) for function evaluation
 
 # 2.2.0
 ## Feature

README.md

@@ -2,6 +2,18 @@ React Editable Json Tree
 ========================
 [![Build Status](https://travis-ci.org/oxyno-zeta/react-editable-json-tree.svg?branch=master)](https://travis-ci.org/oxyno-zeta/react-editable-json-tree)[![Build Status](https://circleci.com/gh/oxyno-zeta/react-editable-json-tree.png)](https://circleci.com/gh/oxyno-zeta/react-editable-json-tree)[![npm](https://img.shields.io/npm/v/react-editable-json-tree.svg)]()
 
+## ⚠ Security advisory
+
+This library was previously affected by an `eval` security vulnerability.
+We have taken steps to mitigate this issue with non-breaking changes in this
+patch, v2.2.2, but for more info, please read
+[our security advisory](https://github.com/oxyno-zeta/react-editable-json-tree/security/advisories/GHSA-j3rv-w43q-f9x2).
+
+If you do not have time to read and want to completely mitigate this issue,
+simply set the [allowFunctionEvaluation](#allowfunctionevaluation)
+prop to `false`. In the next major version, we will set this value to `false` by
+default.
+
 ## Demo
 Demo is available here : [Demo](https://oxyno-zeta.github.io/react-editable-json-tree/)
 
@@ -323,6 +335,12 @@ Function parameters :
 |     key     |    Key of current node/value                             | String  |        'string' for data: { object: { string: 'test' } }                     |
 |   rawValue  | Raw value from inputElement or textareaElement component | String  |        'string' for data: { object: { string: 'test' } }                     |
 
+### allowFunctionEvaluation
+|           Key           |                                               Description                                               |  Type   | Required | Default |
+|:-----------------------:|:-------------------------------------------------------------------------------------------------------:|:-------:|:--------:|:-------:|
+| allowFunctionEvaluation | Allow strings that appear to be Javascript function definitions to be evaluated as Javascript functions | Boolean |  False   |  True   |
+
+
 ## Design
 The library provide CSS class on elements. All are prefixed by "rejt" to avoid conflict. 
 To avoid being linked with a CSS file, the library will use the inline style.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "react-editable-json-tree",
-  "version": "2.2.1",
+  "version": "2.2.2",
   "description": "React Editable Json Tree",
   "main": "dist/JsonTree.js",
   "scripts": {
@@ -21,6 +21,9 @@
     "url": "git@github.com:oxyno-zeta/react-editable-json-tree.git"
   },
   "author": "Oxyno-zeta",
+  "contributors": [
+    "Phanabani"
+  ],
   "license": "MIT",
   "devDependencies": {
     "autoprefixer": "^6.5.3",

src/JsonTree.js

@@ -49,6 +49,7 @@ const propTypes = {
     beforeUpdateAction: PropTypes.func,
     logger: PropTypes.object,
     onSubmitValueParser: PropTypes.func,
+    allowFunctionEvaluation: PropTypes.bool,
 };
 // Default props
 const defaultProps = {
@@ -75,21 +76,41 @@ const defaultProps = {
     beforeAddAction: (key, keyPath, deep, newValue) => new Promise(resolve => resolve()),
     beforeUpdateAction: (key, keyPath, deep, oldValue, newValue) => new Promise(resolve => resolve()),
     logger: { error: () => {} },
-    onSubmitValueParser: (isEditMode, keyPath, deep, name, rawValue) => parse(rawValue),
     inputElement: (usage, keyPath, deep, keyName, data, dataType) => <input />,
     textareaElement: (usage, keyPath, deep, keyName, data, dataType) => <textarea />,
-    /* eslint-enable */
+    /* eslint-enable no-unused-vars */
+    allowFunctionEvaluation: true,
 };
 
+const createParsingFunction = allowFunctionEvaluation =>
+    (isEditMode, keyPath, deep, name, rawValue) =>
+        parse(rawValue, allowFunctionEvaluation);
+
 /* ************************************* */
 /* ********      COMPONENT      ******** */
 /* ************************************* */
 class JsonTree extends Component {
     constructor(props) {
         super(props);
+
+        let onSubmitValueParser;
+        // This WasGenerated value lets us know whether we generated the parsing
+        // function, so we can appropriately react to changes of
+        // `allowFunctionEvaluation`
+        let onSubmitValueParserWasGenerated;
+        if (props.onSubmitValueParser) {
+            onSubmitValueParser = props.onSubmitValueParser;
+            onSubmitValueParserWasGenerated = false;
+        } else {
+            onSubmitValueParser = createParsingFunction(props.allowFunctionEvaluation);
+            onSubmitValueParserWasGenerated = true;
+        }
+
         this.state = {
             data: props.data,
             rootName: props.rootName,
+            onSubmitValueParser,
+            onSubmitValueParserWasGenerated,
         };
         // Bind
         this.onUpdate = this.onUpdate.bind(this);
@@ -100,6 +121,33 @@ class JsonTree extends Component {
             data: nextProps.data,
             rootName: nextProps.rootName,
         });
+
+        let onSubmitValueParserWasGenerated = this.state.onSubmitValueParserWasGenerated;
+        if (
+            nextProps.onSubmitValueParser &&
+            nextProps.onSubmitValueParser !== this.state.onSubmitValueParser
+        ) {
+            // We just added a new submit value parser, so this is definitely
+            // not our default parser anymore
+            onSubmitValueParserWasGenerated = false;
+            this.setState({
+                onSubmitValueParser: nextProps.onSubmitValueParser,
+                onSubmitValueParserWasGenerated,
+            });
+        }
+
+        if (
+            onSubmitValueParserWasGenerated &&
+            nextProps.allowFunctionEvaluation !== this.props.allowFunctionEvaluation
+        ) {
+            // Create a new submit value parser that adheres to the new
+            // `allowFunctionEvaluation` value as long as we know the parser
+            // was generated by us
+            this.setState({
+                onSubmitValueParser:
+                    createParsingFunction(nextProps.allowFunctionEvaluation),
+            });
+        }
     }
 
     onUpdate(key, data) {
@@ -112,7 +160,7 @@ class JsonTree extends Component {
     }
 
     render() {
-        const { data, rootName } = this.state;
+        const { data, rootName, onSubmitValueParser } = this.state;
         const {
             isCollapsed,
             onDeltaUpdate,
@@ -129,7 +177,6 @@ class JsonTree extends Component {
             beforeAddAction,
             beforeUpdateAction,
             logger,
-            onSubmitValueParser,
             } = this.props;
 
         // Node type

src/components/JsonFunctionValue.js

@@ -11,6 +11,7 @@ import PropTypes from 'prop-types';
 import { HotKeys } from 'react-hotkeys';
 import { isComponentWillChange } from '../utils/objectTypes';
 import inputUsageTypes from '../types/inputUsageTypes';
+import { functionToString } from '../utils/parse';
 
 /* ************************************* */
 /* ********      VARIABLES      ******** */
@@ -155,7 +156,7 @@ class JsonFunctionValue extends Component {
             });
             const textareaElementLayout = React.cloneElement(textareaElement, {
                 ref: this.refInput,
-                defaultValue: originalValue,
+                defaultValue: functionToString(originalValue),
             });
 
             result = (<span className="rejt-edit-form" style={style.editForm}>

src/components/JsonNode.js

@@ -14,6 +14,7 @@ import JsonArray from './JsonArray';
 import JsonFunctionValue from './JsonFunctionValue';
 import { getObjectType } from '../utils/objectTypes';
 import dataTypes from '../types/dataTypes';
+import { functionToString } from '../utils/parse';
 
 /* ************************************* */
 /* ********      VARIABLES      ******** */
@@ -292,7 +293,7 @@ class JsonNode extends Component {
             case dataTypes.FUNCTION:
                 return (<JsonFunctionValue
                     name={name}
-                    value={data.toString()}
+                    value={functionToString(data)}
                     originalValue={data}
                     keyPath={keyPath}
                     deep={deep}

src/utils/parse.js

@@ -12,38 +12,151 @@
 /* ********      VARIABLES      ******** */
 /* ************************************* */
 
+const basicFunctionPattern = new RegExp(
+    // eslint-disable-next-line prefer-template
+    ''
+    + /^function/.source
+    + / *([$_a-zA-Z][$\w]*)?/.source // name
+    + / *\([ \n]*([$_a-zA-Z][$\w]*(?:[ \n]*,[ \n]*[$_a-zA-Z][$\w]*)*)*?,?[ \n]*\)/.source // params
+    + /[ \n]*{\n*(.*?)\n? *}$/.source, // body
+    's',
+);
 
 /* ************************************* */
 /* ********  PRIVATE FUNCTIONS  ******** */
 /* ************************************* */
 
+/**
+ * Try to regex match a string as a javascript function.
+ * @param functionString {string} string to match
+ * @param splitParams {boolean} whether to split parameters into an array
+ * @returns {{name: string, params: string | string[], body: string} | null}
+ */
+function matchFunction(functionString, splitParams = false) {
+    const match = basicFunctionPattern.exec(functionString);
+    if (match === null) return null;
+    return {
+        name: match[1],
+        params: splitParams ? commaSplit(match[2]) : match[2],
+        body: match[3],
+    };
+}
+
+/**
+ * Split comma separated strings and trim surrounding whitespace.
+ * @param string {string | undefined} a string of comma-separated strings
+ * @returns {string[]} an array of elements that were separated by commas with
+ *   surrounding whitespace trimmed. May be empty.
+ */
+function commaSplit(string) {
+    if (!string) return [];
+    return string.split(',').map(x => x.trim());
+}
+
+/**
+ * Try creating an anonymous function from a string, or return null if it's
+ * not a valid function definition.
+ * Note that this is not a completely safe, there are still security flaws,
+ * but it is safer than using `exec`.
+ * @param functionString {string} string to try to parse as a function
+ *   definition
+ * @returns {Function | null} an anonymous function if the string is a valid
+ *   function definition, else null
+ */
+function createFunction(functionString) {
+    /* This is not an exhaustive check by any means
+     * For instance, function names may have a wide variety of
+     * unicode characters and still be valid... oh well!
+     *
+     * TEST CASES:
+     *
+     * // Should match (single-line):
+     * function() {}
+     * function () {}
+     * function myFunc(){}
+     * function myFunc(arg1){}
+     * function(arg1,arg2, arg3,  arg4) {}
+     * function myFunc(arg1, arg2, arg3){}
+     * function myFunc(arg1, arg2, arg3){console.log('something');}
+     * function myFunc(arg1,){}
+     * function myFunc(arg1, ){}
+     * function myFunc(arg1) {if (true) {var moreCurlyBraces = 1;}}
+     *
+     * // Should match (multi-line):
+     * function myFunc(arg1, arg2, arg3) {
+     *     console.log('something');
+     * }
+     *
+     * function myFunc() {
+     *     console.log('test');
+     *     if (true) {
+     *         console.log('test2');
+     *     }
+     * }
+     *
+     * // Should not match (single-line):
+     * anotherFunction()
+     * function myFunc {}
+     * function myFunc()); (anotherFunction()
+     * function myFunc(){}, anotherFunction()
+     */
+    const match = matchFunction(functionString, true);
+    if (!match) return null;
+
+    // Here's the security flaw. We want this functionality for supporting
+    // JSONP, so we've opted for the best attempt at maintaining some amount
+    // of security. This should be a little better than eval because it
+    // shouldn't automatically execute code, just create a function which can
+    // be called later.
+    // eslint-disable-next-line no-new-func
+    const func = new Function(...match.params, match.body || '');
+    func.displayName = match.name;
+    return func;
+}
+
 
 /* ************************************* */
 /* ********   PUBLIC FUNCTIONS  ******** */
 /* ************************************* */
 
 /**
- * Parse.
- * @param string {String} string to parse
- * @returns {*}
+ * Parse a string into either a function or a JSON element.
+ * @param string {string} string to parse
+ * @param allowFunctionEvaluation {boolean} whether to parse strings that
+ *   are function definitions as Javascript
+ * @returns {Function | Object | Array | null | boolean | number | string}
  */
-function parse(string) {
-    let result = string;
-
-    // Check if string contains 'function' and start with it to eval it
-    if (result.indexOf('function') === 0) {
-        return eval(`(${result})`); // eslint-disable-line no-eval
+function parse(string, allowFunctionEvaluation) {
+    // Try parsing (and sanitizing) a function
+    if (allowFunctionEvaluation) {
+        const func = createFunction(string);
+        if (func !== null) return func;
     }
 
     try {
-        result = JSON.parse(string);
+        return JSON.parse(string);
     } catch (e) {
-        // Error
+        return string;
+    }
+}
+
+/**
+ * A different implementation of Function.prototype.toString which tries to get
+ * a function name using displayName when name is "anonymous".
+ * @param func {Function} function to transform into a string
+ * @returns {string} a string representing the function
+ */
+function functionToString(func) {
+    const pattern = /^function anonymous/;
+    const funcStr = func.toString();
+    if (pattern.test(funcStr) && func.displayName) {
+        return func.toString().replace(pattern, `function ${func.displayName}`);
     }
-    return result;
+    return funcStr;
 }
 
 /* ************************************* */
 /* ********       EXPORTS       ******** */
 /* ************************************* */
 export default parse;
+export { functionToString };