diff --git a/lib/dao.js b/lib/dao.js index 42fb48b8..ed667538 100644 --- a/lib/dao.js +++ b/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]); } diff --git a/lib/model.js b/lib/model.js index 310c018f..7bacf426 100644 --- a/lib/model.js +++ b/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; diff --git a/lib/utils.js b/lib/utils.js index e374d54d..a23ecb73 100644 --- a/lib/utils.js +++ b/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; }); } diff --git a/test/default-scope.test.js b/test/default-scope.test.js index d2ff2e1a..806904e2 100644 --- a/test/default-scope.test.js +++ b/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() {