fix(security): avoid prototype pollution on model create and update (#1805)

lib/dao.js

@@ -63,6 +63,8 @@ function getIdValue(m, data) {
 
 function copyData(from, to) {
   for (var key in from) {
+    // Don't traverse down prototype chain
+    if (!from.hasOwnProperty(key)) continue;
     to[key] = from[key];
   }
 }
@@ -70,6 +72,8 @@ function copyData(from, to) {
 function convertSubsetOfPropertiesByType(inst, data) {
   var typedData = {};
   for (var key in data) {
+    // Don't traverse down prototype chain
+    if (!data.hasOwnProperty(key)) continue;
     // Convert the properties by type
     typedData[key] = inst[key];
     if (typeof typedData[key] === 'object' &&
@@ -2822,6 +2826,10 @@ DataAccessObject.prototype.remove =
  * @param {Mixed} value Value of property
  */
 DataAccessObject.prototype.setAttribute = function setAttribute(name, value) {
+  // security: Avoid prototype pollution
+  if (name === '__proto__') return;
+  // security: remove __proto__ and undefined if we're setting a nested object
+  if (typeof value === 'object' && value) value = removeUndefined(value);
   this[name] = value; // TODO [fabien] - currently not protected by applyProperties
 };
 
@@ -2855,6 +2863,8 @@ DataAccessObject.prototype.setAttributes = function setAttributes(data) {
 
   // update instance's properties
   for (var key in data) {
+    // Don't traverse down prototype chain
+    if (!data.hasOwnProperty(key)) continue;
     inst.setAttribute(key, data[key]);
   }
 

lib/model.js

@@ -177,6 +177,10 @@ ModelBaseClass.prototype._initProperties = function(data, options) {
   var p, propVal;
   for (var k = 0; k < size; k++) {
     p = keys[k];
+    // security: Avoid prototype pollution
+    if (p === '__proto__') {
+      continue;
+    }
     propVal = data[p];
     if (typeof propVal === 'function') {
       continue;
@@ -257,6 +261,10 @@ ModelBaseClass.prototype._initProperties = function(data, options) {
 
   for (k = 0; k < size; k++) {
     p = keys[k];
+    // security: Avoid prototype pollution
+    if (p === '__proto__') {
+      continue;
+    }
     propVal = self.__data[p];
     var type = properties[p].type;
 

lib/utils.js

@@ -298,7 +298,7 @@ function selectFields(fields) {
 }
 
 /**
- * Remove undefined values from the queury object
+ * Remove undefined values from the query object
  * @param query
  * @param handleUndefined {String} either "nullify", "throw" or "ignore" (default: "ignore")
  * @returns {exports.map|*}
@@ -309,7 +309,11 @@ function removeUndefined(query, handleUndefined) {
   }
   // WARNING: [rfeng] Use map() will cause mongodb to produce invalid BSON
   // as traverse doesn't transform the ObjectId correctly
-  return traverse(query).forEach(function(x) {
+  return traverse(query).forEach(function(x, k) {
+    // security: Avoid prototype pollution
+    if (this.key === '__proto__') {
+      this.delete(true);
+    }
     if (x === undefined) {
       switch (handleUndefined) {
         case 'nullify':
@@ -328,10 +332,7 @@ function removeUndefined(query, handleUndefined) {
         x.constructor !== Object)) {
       // This object is not a plain object
       this.update(x, true); // Stop navigating into this object
-      return x;
     }
-
-    return x;
   });
 }
 

test/default-scope.test.js

@@ -8,7 +8,7 @@
 var should = require('./init.js');
 var async = require('async');
 
-var db, Category, Product, Tool, Widget, Thing, Person;
+var db, Category, Product, Tool, Widget, Thing, Person, FavoriteTool;
 
 // This test requires a connector that can
 // handle a custom collection or table name
@@ -69,6 +69,11 @@ describe('default scope', function() {
       scopes: {active: {where: {active: true}}},
     });
 
+    FavoriteTool = db.define('FavoriteTool', {
+      name: String,
+      tool: {type: 'Tool'},
+    });
+
     Product.lookupModel = function(data) {
       var m = this.dataSource.models[data.kind];
       if (m.base === this) return m;
@@ -211,6 +216,159 @@ describe('default scope', function() {
         done();
       });
     });
+
+    //
+    // Prototype pollution is dangerous. If __proto__ is specified as a regular
+    // key then spread over the object, you can risk overwriting Object.prototype.
+    //
+    // It's not possible to do this by passing an object literal, as __proto__ will not
+    // be enumerable. But it *is* possible to do this when it comes in through JSON.parse()
+    // (i.e. through body-parser or the equivalent), which can lead to crazy errors,
+    // where the underlying ModelConstructor gets overridden and interal methods disappear.
+    //
+    // You'll see errors like `this.trigger is not a function`.
+    //
+    // At that point, anything goes. So we need to block it anywhere we might take input.
+    //
+
+    it('security: prototype pollution - updateAttributes', function(done) {
+      Tool.create({name: 'Product A'}, function(err, p) {
+        p.updateAttributes({__proto__: {'evil': 'foo'}, good: 'bar'}, function(err, inst) {
+          should.not.exist(err);
+          should.not.exist(inst.evil);
+          should.not.exist(inst.__proto__.evil);
+          inst.good.should.equal('bar');
+          done();
+        });
+      });
+    });
+
+    // Really dangerous: this will overwrite ModelConstructor attributes
+    // So this won't even save, it'll throw a crazy error while it attempts to validate
+    it('security: prototype pollution - updateAttributes with JSON.parse()', function(done) {
+      Tool.create({name: 'Product A'}, function(err, p) {
+        p.updateAttributes(JSON.parse('{"__proto__": {"evil": "foo"}, "good": "bar"}'), function(err, inst) {
+          should.not.exist(err);
+          should.not.exist(inst.evil);
+          inst.good.should.equal('bar');
+          done();
+        });
+      });
+    });
+
+    it('security: prototype pollution - updateAttributes on nested object', function(done) {
+      FavoriteTool.create({name: 'Product A', tool: {name: 'Product A'}}, function(err, p) {
+        p.updateAttributes({tool: {__proto__: {evil: 'foo'}, good: 'bar'}}, function(err, inst) {
+          should.not.exist(err);
+          should.not.exist(inst.tool.evil);
+          inst.tool.good.should.equal('bar');
+          done();
+        });
+      });
+    });
+
+    it('security: prototype pollution - updateAttributes on nested object with JSON.parse()', function(done) {
+      FavoriteTool.create({name: 'Product A', tool: {name: 'Product A'}}, function(err, p) {
+        p.updateAttributes(JSON.parse('{"tool": {"__proto__": {"evil": "foo"}, "good": "bar"}}'),
+        function(err, inst) {
+          should.not.exist(err);
+          should.not.exist(inst.tool.evil);
+          inst.tool.good.should.equal('bar');
+          done();
+        });
+      });
+    });
+
+    it('security: prototype pollution - constructor', function() {
+      var p = new Tool({__proto__: {bar: 'danger'}});
+      should.not.exist(p.__proto__.bar);
+      should.not.exist(p.bar);
+    });
+
+    it('security: prototype pollution - constructor with JSON.parse()', function() {
+      var p = new Tool(JSON.parse('{"__proto__": {"bar": "danger"}}'));
+      should.not.exist(p.__proto__.bar);
+      should.not.exist(p.bar);
+    });
+
+    it('security: prototype pollution - setAttribute', function() {
+      var p = new Tool();
+      p.setAttributes({__proto__: {bar: 'danger'}});
+      should.not.exist(p.__proto__.bar);
+      should.not.exist(p.bar);
+      p.setAttribute('__proto__', {bar: 'danger'});
+      should.not.exist(p.__proto__.bar);
+      should.not.exist(p.bar);
+    });
+
+    it('security: prototype pollution - setAttribute with JSON.parse()', function() {
+      var p = new Tool();
+      p.setAttributes(JSON.parse('{"__proto__": {"bar": "danger"}}'));
+      should.not.exist(p.__proto__.bar);
+      should.not.exist(p.bar);
+    });
+
+    it('security: prototype pollution - nested setAttribute', function() {
+      var p = new Tool();
+      p.setAttributes({baz: {__proto__: {bar: 'danger'}}});
+      // should not leak to model prototype
+      should.not.exist(p.__proto__.bar);
+      should.not.exist(p.bar);
+      // But should be on nested object on prototype, as we created a raw
+      // object and __proto__ is not enumerable, and thus won't be removed.
+      p.baz.bar.should.equal('danger');
+      p.baz.__proto__.bar.should.equal('danger');
+
+      p.setAttribute('biff', {__proto__: {bar: 'danger'}});
+      should.not.exist(p.__proto__.bar);
+      should.not.exist(p.bar);
+      // But should be on nested object on prototype
+      p.biff.bar.should.equal('danger');
+      p.biff.__proto__.bar.should.equal('danger');
+    });
+
+    it('security: prototype pollution - nested setAttribute with JSON.parse()', function() {
+      var p = new Tool();
+      p.setAttributes({baz: JSON.parse('{"__proto__": {"bar": "danger"}}')});
+      // should not leak to model prototype or nested object's prototype
+      should.not.exist(p.__proto__.bar);
+      should.not.exist(p.bar);
+      // and should not be on nested object
+      should.not.exist(p.baz.bar);
+      should.not.exist(p.baz.__proto__.bar);
+
+      p.setAttribute('biff', JSON.parse('{"__proto__": {"bar": "danger"}}'));
+      should.not.exist(p.__proto__.bar);
+      should.not.exist(p.bar);
+      // and should not be on nested object
+      should.not.exist(p.biff.bar);
+      should.not.exist(p.baz.__proto__.bar);
+    });
+
+    it('security: prototype pollution - create', function(done) {
+      Tool.create({foo: {__proto__: {bar: 'danger'}}}, function(err, p) {
+        should.not.exist(err);
+        should.not.exist(p.__proto__.bar);
+        should.not.exist(p.bar);
+        // Will be on nested object's proto
+        p.foo.bar.should.equal('danger');
+        p.foo.__proto__.bar.should.equal('danger');
+        done();
+      });
+    });
+
+    it('security: prototype pollution - create with JSON.parse()', function(done) {
+      Tool.create({foo: JSON.parse('{"__proto__": {"bar": "danger"}}')}, function(err, p) {
+        should.not.exist(err);
+        should.not.exist(p.__proto__.bar);
+        should.not.exist(p.bar);
+        // Will be on nested object's as enumerable key
+        should.not.exist(p.foo.bar);
+        // Removed in removeUndefined() called in create
+        should.not.exist(p.foo.__proto__.bar);
+        done();
+      });
+    });
   });
 
   describe('findById', function() {