Merge pull request #219 from Starcounter-Jack/ban-proto-edits

Ban prototype pollution and document it

.travis.yml

@@ -3,5 +3,7 @@ dist: trusty
 before_script:
   - npm install
 node_js: 8
+before_script: 
+  - npm install
 script:
   - npm test && npm run bench

README.md

@@ -182,42 +182,50 @@ else {
 
 ## API
 
-#### `jsonpatch.applyPatch<T>(document: any, patch: Operation[], validateOperation: Boolean | Function = false): OperationResult<T>[]`
+#### `function applyPatch<T>(document: T, patch: Operation[], validateOperation?: boolean | Validator<T>, mutateDocument: boolean = true, banPrototypeModifications: boolean = true): PatchResult<T>`
 
 Applies `patch` array on `obj`.
 
+- `document` The document to patch
+- `patch` a JSON-Patch array of operations to apply
+- `validateOperation` Boolean for whether to validate each operation with our default validator, or to pass a validator callback
+- `mutateDocument` Whether to mutate the original document or clone it before applying
+- `banPrototypeModifications`  Whether to ban modifications to `__proto__`, defaults to `true`.
+
 An invalid patch results in throwing an error (see `jsonpatch.validate` for more information about the error object).
 
 It modifies the `document` object and `patch` - it gets the values by reference.
-If you would like to avoid touching your values, clone them: `jsonpatch.applyPatch(document, jsonpatch.deepClone(patch))`.
+If you would like to avoid touching your `patch` array values, clone them: `jsonpatch.applyPatch(document, jsonpatch.deepClone(patch))`.
 
 Returns an array of [`OperationResult`](#operationresult-type) objects - one item for each item in `patches`, each item is an object `{newDocument: any, test?: boolean, removed?: any}`.
 
 * `test` - boolean result of the test
 * `remove`, `replace` and `move` - original object that has been removed
 * `add` (only when adding to an array) - index at which item has been inserted (useful when using `-` alias)
 
-** Note: It throws `TEST_OPERATION_FAILED` error if `test` operation fails. **
-
-** Note II: the returned array has `newDocument` property that you can use as the final state of the patched document **.
+- ** Note: It throws `TEST_OPERATION_FAILED` error if `test` operation fails. **
+- ** Note II: the returned array has `newDocument` property that you can use as the final state of the patched document **.
+- ** Note III: By default, when `banPrototypeModifications` is `true`, this method throws a `TypeError` when you attempt to modify an object's prototype.
 
 - See [Validation notes](#validation-notes).
 
-#### `applyOperation<T>(document: any, operation: Operation, validateOperation: <Boolean | Function> = false, mutateDocument = true): OperationResult<T>`
+#### `function applyOperation<T>(document: T, operation: Operation, validateOperation: boolean | Validator<T> = false, mutateDocument: boolean = true, banPrototypeModifications: boolean = true): OperationResult<T>`
 
 Applies single operation object `operation` on `document`.
 
 - `document` The document to patch
 - `operation` The operation to apply
 - `validateOperation` Whether to validate the operation, or to pass a validator callback
 - `mutateDocument` Whether to mutate the original document or clone it before applying
+- `banPrototypeModifications` Whether to ban modifications to `__proto__`, defaults to `true`.
 
 It modifies the `document` object and `operation` - it gets the values by reference.
 If you would like to avoid touching your values, clone them: `jsonpatch.applyOperation(document, jsonpatch.deepClone(operation))`.
 
 Returns an [`OperationResult`](#operationresult-type) object `{newDocument: any, test?: boolean, removed?: any}`.
 
-** Note: It throws `TEST_OPERATION_FAILED` error if `test` operation fails. **
+- ** Note: It throws `TEST_OPERATION_FAILED` error if `test` operation fails. **
+- ** Note II: By default, when `banPrototypeModifications` is `true`, this method throws a `TypeError` when you attempt to modify an object's prototype.
 
 - See [Validation notes](#validation-notes).
 

dist/fast-json-patch.js

@@ -347,11 +347,13 @@ exports.getValueByPointer = getValueByPointer;
  * @param operation The operation to apply
  * @param validateOperation `false` is without validation, `true` to use default jsonpatch's validation, or you can pass a `validateOperation` callback to be used for validation.
  * @param mutateDocument Whether to mutate the original document or clone it before applying
+ * @param banPrototypeModifications Whether to ban modifications to `__proto__`, defaults to `true`.
  * @return `{newDocument, result}` after the operation
  */
-function applyOperation(document, operation, validateOperation, mutateDocument) {
+function applyOperation(document, operation, validateOperation, mutateDocument, banPrototypeModifications) {
     if (validateOperation === void 0) { validateOperation = false; }
     if (mutateDocument === void 0) { mutateDocument = true; }
+    if (banPrototypeModifications === void 0) { banPrototypeModifications = true; }
     if (validateOperation) {
         if (typeof validateOperation == 'function') {
             validateOperation(operation, 0, document, operation.path);
@@ -425,6 +427,9 @@ function applyOperation(document, operation, validateOperation, mutateDocument)
         }
         while (true) {
             key = keys[t];
+            if (banPrototypeModifications && key == '__proto__') {
+                throw new TypeError('JSON-Patch: modifying `__proto_` prop is banned for security reasons, if this was on purpose, please set `banPrototypeModifications` flag false and pass it to this function. More info in fast-json-patch README');
+            }
             if (validateOperation) {
                 if (existingPathFragment === undefined) {
                     if (obj[key] === undefined) {
@@ -490,10 +495,12 @@ exports.applyOperation = applyOperation;
  * @param patch The patch to apply
  * @param validateOperation `false` is without validation, `true` to use default jsonpatch's validation, or you can pass a `validateOperation` callback to be used for validation.
  * @param mutateDocument Whether to mutate the original document or clone it before applying
+ * @param banPrototypeModifications Whether to ban modifications to `__proto__`, defaults to `true`.
  * @return An array of `{newDocument, result}` after the patch
  */
-function applyPatch(document, patch, validateOperation, mutateDocument) {
+function applyPatch(document, patch, validateOperation, mutateDocument, banPrototypeModifications) {
     if (mutateDocument === void 0) { mutateDocument = true; }
+    if (banPrototypeModifications === void 0) { banPrototypeModifications = true; }
     if (validateOperation) {
         if (!Array.isArray(patch)) {
             throw new exports.JsonPatchError('Patch sequence must be an array', 'SEQUENCE_NOT_AN_ARRAY');
@@ -504,7 +511,7 @@ function applyPatch(document, patch, validateOperation, mutateDocument) {
     }
     var results = new Array(patch.length);
     for (var i = 0, length_1 = patch.length; i < length_1; i++) {
-        results[i] = applyOperation(document, patch[i], validateOperation);
+        results[i] = applyOperation(document, patch[i], validateOperation, true, banPrototypeModifications);
         document = results[i].newDocument; // in case root was replaced
     }
     results.newDocument = document;

src/core.ts

@@ -15,11 +15,6 @@ import { PatchError, _deepClone, isInteger, unescapePathComponent, hasUndefined
 export const JsonPatchError = PatchError;
 export const deepClone = _deepClone;
 
-interface HTMLElement {
-  attachEvent: Function;
-  detachEvent: Function;
-}
-
 export type Operation = AddOperation<any> | RemoveOperation | ReplaceOperation<any> | MoveOperation | CopyOperation | TestOperation<any> | GetOperation<any>;
 
 export interface Validator<T> {
@@ -193,9 +188,10 @@ export function getValueByPointer(document: any, pointer: string): any {
  * @param operation The operation to apply
  * @param validateOperation `false` is without validation, `true` to use default jsonpatch's validation, or you can pass a `validateOperation` callback to be used for validation.
  * @param mutateDocument Whether to mutate the original document or clone it before applying
+ * @param banPrototypeModifications Whether to ban modifications to `__proto__`, defaults to `true`.
  * @return `{newDocument, result}` after the operation
  */
-export function applyOperation<T>(document: T, operation: Operation, validateOperation: boolean | Validator<T> = false, mutateDocument: boolean = true): OperationResult<T> {
+export function applyOperation<T>(document: T, operation: Operation, validateOperation: boolean | Validator<T> = false, mutateDocument: boolean = true, banPrototypeModifications: boolean = true): OperationResult<T> {
   if (validateOperation) {
     if (typeof validateOperation == 'function') {
       validateOperation(operation, 0, document, operation.path);
@@ -264,6 +260,10 @@ export function applyOperation<T>(document: T, operation: Operation, validateOpe
     while (true) {
       key = keys[t];
 
+      if(banPrototypeModifications && key == '__proto__') {
+        throw new TypeError('JSON-Patch: modifying `__proto__` prop is banned for security reasons, if this was on purpose, please set `banPrototypeModifications` flag false and pass it to this function. More info in fast-json-patch README');
+      }
+
       if (validateOperation) {
         if (existingPathFragment === undefined) {
           if (obj[key] === undefined) {
@@ -329,9 +329,10 @@ export function applyOperation<T>(document: T, operation: Operation, validateOpe
  * @param patch The patch to apply
  * @param validateOperation `false` is without validation, `true` to use default jsonpatch's validation, or you can pass a `validateOperation` callback to be used for validation.
  * @param mutateDocument Whether to mutate the original document or clone it before applying
+ * @param banPrototypeModifications Whether to ban modifications to `__proto__`, defaults to `true`.
  * @return An array of `{newDocument, result}` after the patch
  */
-export function applyPatch<T>(document: T, patch: Operation[], validateOperation?: boolean | Validator<T>, mutateDocument: boolean = true): PatchResult<T> {
+export function applyPatch<T>(document: T, patch: Operation[], validateOperation?: boolean | Validator<T>, mutateDocument: boolean = true, banPrototypeModifications: boolean = true): PatchResult<T> {
   if(validateOperation) {
     if(!Array.isArray(patch)) {
       throw new JsonPatchError('Patch sequence must be an array', 'SEQUENCE_NOT_AN_ARRAY');
@@ -343,7 +344,8 @@ export function applyPatch<T>(document: T, patch: Operation[], validateOperation
   const results = new Array(patch.length) as PatchResult<T>;
 
   for (let i = 0, length = patch.length; i < length; i++) {
-    results[i] = applyOperation(document, patch[i], validateOperation);
+    // we don't need to pass mutateDocument argument because if it was true, we already deep cloned the object, we'll just pass `true`
+    results[i] = applyOperation(document, patch[i], validateOperation, true, banPrototypeModifications);
     document = results[i].newDocument; // in case root was replaced
   }
   results.newDocument = document;

src/duplex.ts

@@ -3,15 +3,8 @@
  * (c) 2017 Joachim Wester
  * MIT license
  */
-declare var require: any;
-
-const equalsOptions = { strict: true };
-const _equals = require('deep-equal');
-const areEquals = (a: any, b: any): boolean => {
-  return _equals(a, b, equalsOptions)
-}
-import { PatchError, _deepClone, isInteger, _objectKeys, escapePathComponent, unescapePathComponent, hasUndefined, hasOwnProperty } from './helpers';
-import { applyOperation, applyPatch, getValueByPointer, Operation } from './core';
+import { _deepClone, _objectKeys, escapePathComponent, hasOwnProperty } from './helpers';
+import { applyPatch, Operation } from './core';
 
 /* export all core functions */
 export { applyOperation, applyPatch, applyReducer, getValueByPointer, Operation, validate, validator, OperationResult } from './core';

test/spec/coreSpec.js

@@ -9,40 +9,44 @@ if (typeof _ === 'undefined') {
 describe('jsonpatch.getValueByPointer', function() {
   it('should retrieve values by JSON pointer from tree - deep object', function() {
     var obj = {
-      person: {name: 'Marilyn'}
+      person: { name: 'Marilyn' }
     };
-    var name = jsonpatch.getValueByPointer(obj, '/person/name')
+    var name = jsonpatch.getValueByPointer(obj, '/person/name');
     expect(name).toEqual('Marilyn');
   });
 
   it('should retrieve values by JSON pointer from tree - deep array', function() {
     var obj = {
-      people: [{name: 'Marilyn'}, {name: 'Monroe'}]
+      people: [{ name: 'Marilyn' }, { name: 'Monroe' }]
     };
-    var name = jsonpatch.getValueByPointer(obj, '/people/1/name')
+    var name = jsonpatch.getValueByPointer(obj, '/people/1/name');
     expect(name).toEqual('Monroe');
   });
 
   it('should retrieve values by JSON pointer from tree - root object', function() {
     var obj = {
-      people: [{name: 'Marilyn'}, {name: 'Monroe'}]
+      people: [{ name: 'Marilyn' }, { name: 'Monroe' }]
     };
     var retrievedObject = jsonpatch.getValueByPointer(obj, '');
 
     expect(retrievedObject).toEqual({
-      people: [{name: 'Marilyn'}, {name: 'Monroe'}]
+      people: [{ name: 'Marilyn' }, { name: 'Monroe' }]
     });
   });
 
   it('should retrieve values by JSON pointer from tree - root array', function() {
-    var obj = [{
-      people: [{name: 'Marilyn'}, {name: 'Monroe'}]
-    }];
+    var obj = [
+      {
+        people: [{ name: 'Marilyn' }, { name: 'Monroe' }]
+      }
+    ];
     var retrievedObject = jsonpatch.getValueByPointer(obj, '');
 
-    expect(retrievedObject).toEqual([{
-      people: [{name: 'Marilyn'}, {name: 'Monroe'}]
-    }]);
+    expect(retrievedObject).toEqual([
+      {
+        people: [{ name: 'Marilyn' }, { name: 'Monroe' }]
+      }
+    ]);
   });
 });
 describe('jsonpatch.applyReducer - using with Array#reduce', function() {
@@ -62,26 +66,30 @@ describe('jsonpatch.applyReducer - using with Array#reduce', function() {
   });
 });
 describe('root replacement with applyOperation', function() {
-  describe('_get operation', function () {
+  describe('_get operation', function() {
     it('should get root value', function() {
-      var obj = [{
-        people: [{name: 'Marilyn'}, {name: 'Monroe'}]
-      }];
+      var obj = [
+        {
+          people: [{ name: 'Marilyn' }, { name: 'Monroe' }]
+        }
+      ];
 
-      var patch = {op: '_get', path: ''};
+      var patch = { op: '_get', path: '' };
 
       jsonpatch.applyOperation(obj, patch);
 
-      expect(patch.value).toEqual([{
-        people: [{name: 'Marilyn'}, {name: 'Monroe'}]
-      }]);
+      expect(patch.value).toEqual([
+        {
+          people: [{ name: 'Marilyn' }, { name: 'Monroe' }]
+        }
+      ]);
     });
     it('should get deep value', function() {
       var obj = {
-        people: [{name: 'Marilyn'}, {name: 'Monroe'}]
+        people: [{ name: 'Marilyn' }, { name: 'Monroe' }]
       };
 
-      var patch = {op: '_get', path: '/people/1/name'};
+      var patch = { op: '_get', path: '/people/1/name' };
 
       jsonpatch.applyOperation(obj, patch);
 
@@ -179,7 +187,7 @@ describe('root replacement with applyOperation', function() {
         }
       ]);
     });
-     it('should `add` an array prop', function() {
+    it('should `add` an array prop', function() {
       var obj = [];
 
       var newObj = jsonpatch.applyOperation(obj, {
@@ -1508,25 +1516,23 @@ describe('core', function() {
   it('should apply copy, without leaving cross-reference between nodes', function() {
     var obj = {};
     var patchset = [
-      {op: 'add', path: '/foo', value: []},
-      {op: 'add', path: '/foo/-', value: 1},
-      {op: 'copy', from: '/foo', path: '/bar'},
-      {op: 'add', path: '/bar/-', value: 2}
+      { op: 'add', path: '/foo', value: [] },
+      { op: 'add', path: '/foo/-', value: 1 },
+      { op: 'copy', from: '/foo', path: '/bar' },
+      { op: 'add', path: '/bar/-', value: 2 }
     ];
 
     jsonpatch.applyPatch(obj, patchset);
 
     expect(obj).toEqual({
-      "foo": [1],
-      "bar": [1, 2],
+      foo: [1],
+      bar: [1, 2]
     });
   });
 
-  it('should use value object as a reference', function () {
+  it('should use value object as a reference', function() {
     var obj1 = {};
-    var patch = [
-      {op: 'add', path: '/foo', value: []}
-    ];
+    var patch = [{ op: 'add', path: '/foo', value: [] }];
 
     jsonpatch.applyPatch(obj1, patch, false);
 
@@ -1910,5 +1916,47 @@ describe('undefined - JS to JSON projection / JSON to JS extension', function()
         bar: null
       });
     });
+
+    it(`should allow __proto__ modifications when the banPrototypeModifications flag is set`, function() {
+      function SomeClass() {
+        this.foo = 'bar';
+      }
+
+      let doc = new SomeClass();
+      let otherDoc = new SomeClass();
+
+      const patch = [
+        { op: 'replace', path: `/__proto__/x`, value: 'polluted' }
+      ];
+
+      jsonpatch.applyPatch(doc, patch, false, true, false);
+
+      expect(otherDoc.x).toEqual('polluted');
+    });
+
+    it(`should not allow __proto__ modifications without unsetting the banPrototypeModifications flag and should throw an error`, function() {
+      const expectedErrorMessage =
+        'JSON-Patch: modifying `__proto__` prop is banned for security reasons, if this was on purpose, please set `banPrototypeModifications` flag false and pass it to this function. More info in fast-json-patch README';
+
+      function SomeClass() {
+        this.foo = 'bar';
+      }
+
+      let doc = new SomeClass();
+      let otherDoc = new SomeClass();
+
+      const patch = [
+        { op: 'replace', path: `/__proto__/x`, value: 'polluted' }
+      ];
+
+      expect(() => jsonpatch.applyPatch(doc, patch)).toThrow(new TypeError(expectedErrorMessage));
+
+      expect(otherDoc.x).toEqual(undefined);
+      expect(doc.x).toEqual(undefined);
+
+      let arr = [];
+      expect(() => jsonpatch.applyPatch(arr, patch)).toThrow(new TypeError(expectedErrorMessage));
+      expect(arr.x).toEqual(undefined);
+    });
   });
 });