Merge pull request #12719 from zzztttkkk/master

make `modifiedPaths` safer.

lib/helpers/common.js

@@ -7,6 +7,8 @@
 const Binary = require('../driver').get().Binary;
 const isBsonType = require('./isBsonType');
 const isMongooseObject = require('./isMongooseObject');
+const MongooseError = require('../error');
+const util = require('util');
 
 exports.flatten = flatten;
 exports.modifiedPaths = modifiedPaths;
@@ -67,7 +69,25 @@ function flatten(update, path, options, schema) {
  * ignore
  */
 
-function modifiedPaths(update, path, result) {
+function modifiedPaths(update, path, result, recursion = null) {
+  if (update == null || typeof update !== 'object') {
+    return;
+  }
+
+  if (recursion == null) {
+    recursion = {
+      raw: { update, path },
+      trace: new WeakSet()
+    };
+  }
+
+  if (recursion.trace.has(update)) {
+    throw new MongooseError(`a circular reference in the update value, updateValue:
+${util.inspect(recursion.raw.update, { showHidden: false, depth: 1 })}
+updatePath: '${recursion.raw.path}'`);
+  }
+  recursion.trace.add(update);
+
   const keys = Object.keys(update || {});
   const numKeys = keys.length;
   result = result || {};
@@ -83,7 +103,7 @@ function modifiedPaths(update, path, result) {
       val = val.toObject({ transform: false, virtuals: false });
     }
     if (shouldFlatten(val)) {
-      modifiedPaths(val, path + key, result);
+      modifiedPaths(val, path + key, result, recursion);
     }
   }
 
@@ -96,11 +116,11 @@ function modifiedPaths(update, path, result) {
 
 function shouldFlatten(val) {
   return val &&
-    typeof val === 'object' &&
-    !(val instanceof Date) &&
-    !isBsonType(val, 'ObjectID') &&
-    (!Array.isArray(val) || val.length !== 0) &&
-    !(val instanceof Buffer) &&
-    !isBsonType(val, 'Decimal128') &&
-    !(val instanceof Binary);
+      typeof val === 'object' &&
+      !(val instanceof Date) &&
+      !isBsonType(val, 'ObjectID') &&
+      (!Array.isArray(val) || val.length !== 0) &&
+      !(val instanceof Buffer) &&
+      !isBsonType(val, 'Decimal128') &&
+      !(val instanceof Binary);
 }

test/helpers/common.test.js

@@ -0,0 +1,22 @@
+'use strict';
+
+const assert = require('assert');
+const start = require('../common');
+
+const modifiedPaths = require('../../lib/helpers/common').modifiedPaths;
+const mongoose = start.mongoose;
+const { Schema } = mongoose;
+
+
+describe('modifiedPaths, bad update value which has circular reference field', () => {
+  it('update value can be null', function() {
+    modifiedPaths(null, 'path', null);
+  });
+
+  it('values with obvious error on circular reference', function() {
+    const objA = {};
+    objA.a = objA;
+
+    assert.throws(() => modifiedPaths(objA, 'path', null), /circular reference/);
+  });
+});