From f2e718639a43aca14ab06e8bf5340434fca5d3e8 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 12 Oct 2018 12:40:33 -0700 Subject: [PATCH 1/5] Prevent hidden/protected props from being searched - report errors if where contains hidden properties - report errors if incldue scope contains hidden or protected properties --- lib/dao.js | 27 +++++++ lib/include.js | 6 ++ lib/utils.js | 37 +++++++++- test/model-definition.test.js | 135 +++++++++++++++++++++++++++++++++- 4 files changed, 199 insertions(+), 6 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 8a232c80..1faff391 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -1643,6 +1643,26 @@ function coerceArray(val) { return arrayVal; } +DataAccessObject._getHiddenProperties = function() { + var settings = this.definition.settings || {}; + var result = settings.hiddenProperties || settings.hidden; + if (typeof result === 'string') { + result = [result]; + } + result = result || []; + return result; +}; + +DataAccessObject._getProtectedProperties = function() { + var settings = this.definition.settings || {}; + var result = settings.protectedProperties || settings.protected; + if (typeof result === 'string') { + result = [result]; + } + result = result || []; + return result; +}; + /* * Coerce values based the property types * @param {Object} where The where clause @@ -1659,6 +1679,13 @@ DataAccessObject._coerce = function(where, options) { options = options || {}; + // Check violation of keys + var prohibitedKeys = this._getHiddenProperties(); + if (options.excludeProtectedProperties) { + prohibitedKeys = prohibitedKeys.concat(this._getProtectedProperties()); + } + utils.validateKeys(where, prohibitedKeys); + var err; if (typeof where !== 'object' || Array.isArray(where)) { err = new Error(g.f('The where clause %j is not an {{object}}', where)); diff --git a/lib/include.js b/lib/include.js index 195c5e3a..18e94f37 100644 --- a/lib/include.js +++ b/lib/include.js @@ -204,6 +204,12 @@ Inclusion.include = function(objects, include, options, cb) { * @param cb */ function findWithForeignKeysByPage(model, filter, fkName, pageSize, options, cb) { + try { + model._coerce(filter.where, {excludeProtectedProperties: true}); + } catch (e) { + return cb(e); + } + var foreignKeys = []; if (filter.where[fkName]) { foreignKeys = filter.where[fkName].inq; diff --git a/lib/utils.js b/lib/utils.js index fe8e0e42..9e67123e 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -27,6 +27,7 @@ exports.collectTargetIds = collectTargetIds; exports.idName = idName; exports.rankArrayElements = rankArrayElements; exports.idsHaveDuplicates = idsHaveDuplicates; +exports.validateKeys = validateKeys; var g = require('strong-globalize')(); var traverse = require('traverse'); @@ -302,7 +303,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|*} @@ -339,6 +340,38 @@ function removeUndefined(query, handleUndefined) { }); } +/** + * Check the where clause to report prohibited keys + * @param {object} where Where object + * @param {string[]} [prohibitedKeys] An array of prohibited keys + */ +function validateKeys(where, prohibitedKeys) { + if (!prohibitedKeys || prohibitedKeys.length === 0) return where; + if (typeof where !== 'object' || where === null) { + return where; + } + prohibitedKeys = prohibitedKeys || []; + const offendingKeys = []; + // WARNING: [rfeng] Use traverse.map() will cause mongodb to produce invalid BSON + // as traverse doesn't transform the ObjectId correctly. Instead we have to use + // traverse(where).forEach(...) + const result = traverse(where).forEach(function(x) { + if (prohibitedKeys.indexOf(this.key) !== -1) { + offendingKeys.push(this.key); + } + return x; + }); + if (offendingKeys.length) { + const msg = 'Properties "' + offendingKeys.join(', ') + '" are not allowed in query'; + const err = new Error(msg); + err.code = 'PROPERTY_NOT_ALLOWED_IN_QUERY'; + err.statusCode = 400; + err.details = {properties: offendingKeys, where: where}; + throw err; + } + return result; +} + var url = require('url'); var qs = require('qs'); @@ -505,7 +538,7 @@ function defineCachedRelations(obj) { /** * Check if the argument is plain object - * @param {*) obj The obj value + * @param {*} obj The obj value * @returns {boolean} */ function isPlainObject(obj) { diff --git a/test/model-definition.test.js b/test/model-definition.test.js index bf7a0e97..b0e025de 100644 --- a/test/model-definition.test.js +++ b/test/model-definition.test.js @@ -258,7 +258,6 @@ describe('ModelDefinition class', function() { }); it('should serialize protected properties into JSON', function() { - var modelBuilder = memory.modelBuilder; var ProtectedModel = memory.createModel('protected', {}, { protected: ['protectedProperty'], }); @@ -272,7 +271,6 @@ describe('ModelDefinition class', function() { }); it('should not serialize protected properties of nested models into JSON', function(done) { - var modelBuilder = memory.modelBuilder; var Parent = memory.createModel('parent'); var Child = memory.createModel('child', {}, {protected: ['protectedProperty']}); Parent.hasMany(Child); @@ -295,7 +293,6 @@ describe('ModelDefinition class', function() { }); it('should not serialize hidden properties into JSON', function() { - var modelBuilder = memory.modelBuilder; var HiddenModel = memory.createModel('hidden', {}, { hidden: ['secret'], }); @@ -312,7 +309,6 @@ describe('ModelDefinition class', function() { }); it('should not serialize hidden properties of nested models into JSON', function(done) { - var modelBuilder = memory.modelBuilder; var Parent = memory.createModel('parent'); var Child = memory.createModel('child', {}, {hidden: ['secret']}); Parent.hasMany(Child); @@ -334,6 +330,137 @@ describe('ModelDefinition class', function() { }); }); + function assertPropertyNotAllowed(err) { + should.exist(err); + err.message.should.match(/Properties "secret" are not allowed in query/); + err.code.should.equal('PROPERTY_NOT_ALLOWED_IN_QUERY'); + err.statusCode.should.equal(400); + err.details.should.have.property('properties'); + err.details.should.have.property('where'); + } + + it('should report errors if a hidden property is used in where', function(done) { + var Child = memory.createModel('child', {}, {hidden: ['secret']}); + Child.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Child.find({ + where: {secret: 'secret'}, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + + it('should report errors if a hidden property is used in where.and', function(done) { + var Child = memory.createModel('child', {}, {hidden: ['secret']}); + Child.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Child.find({ + where: {and: [{secret: 'secret'}]}, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + + it('should report errors if a protected property is used in include scope', function(done) { + var Parent = memory.createModel('parent'); + var Child = memory.createModel('child', {}, {protected: ['secret']}); + Parent.hasMany(Child); + Parent.create({ + name: 'parent', + }, function(err, parent) { + if (err) return done(err); + parent.children.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', + }, + }, + }, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + }); + + it('should report errors if a protected property is used in include scope.where.and', function(done) { + var Parent = memory.createModel('parent'); + var Child = memory.createModel('child', {}, {protected: ['secret']}); + Parent.hasMany(Child); + Parent.create({ + name: 'parent', + }, function(err, parent) { + if (err) return done(err); + parent.children.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Parent.find({ + include: { + relation: 'children', + scope: { + where: { + and: [{secret: 'x'}], + }, + }, + }, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + }); + + it('should report errors if a hidden property is used in include scope', function(done) { + var Parent = memory.createModel('parent'); + var Child = memory.createModel('child', {}, {hidden: ['secret']}); + Parent.hasMany(Child); + Parent.create({ + name: 'parent', + }, function(err, parent) { + if (err) return done(err); + parent.children.create({ + name: 'child', + secret: 'secret', + }, function(err) { + if (err) return done(err); + Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', + }, + }, + }, + }, function(err) { + assertPropertyNotAllowed(err); + done(); + }); + }); + }); + }); + it('should throw error for property names containing dot', function() { (function() { memory.createModel('Dotted', {'dot.name': String}); }) .should From a761e0d114486ea547ad0ae4391cfa3faccc9580 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Sun, 14 Oct 2018 09:27:46 -0700 Subject: [PATCH 2/5] Tidy up extended operator check --- lib/dao.js | 21 ++++++++- test/allow-extended-operators.test.js | 63 +++++++++++++++------------ 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 1faff391..f119403b 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -1477,6 +1477,7 @@ DataAccessObject._getSetting = function(key) { }; var operators = { + eq: '=', gt: '>', gte: '>=', lt: '<', @@ -1818,6 +1819,7 @@ DataAccessObject._coerce = function(where, options) { // NOOP when not coercable into an array. } + var allowExtendedOperators = self._allowExtendedOperators(options); // Coerce the array items if (Array.isArray(val)) { for (var i = 0; i < val.length; i++) { @@ -1829,7 +1831,6 @@ DataAccessObject._coerce = function(where, options) { } } else { if (val != null) { - var allowExtendedOperators = self._allowExtendedOperators(options); if (operator === null && val instanceof RegExp) { // Normalize {name: /A/} to {name: {regexp: /A/}} operator = 'regexp'; @@ -1841,12 +1842,28 @@ DataAccessObject._coerce = function(where, options) { } else if (allowExtendedOperators && typeof val === 'object') { // Do not coerce object values when extended operators are allowed } else { + if (!allowExtendedOperators) { + var extendedOperators = Object.keys(val).filter(function(k) { + return k[0] === '$'; + }); + if (extendedOperators.length) { + const msg = g.f('Operators "' + extendedOperators.join(', ') + '" are not allowed in query'); + const err = new Error(msg); + err.code = 'OPERATOR_NOT_ALLOWED_IN_QUERY'; + err.statusCode = 400; + err.details = { + operators: extendedOperators, + where: where, + }; + throw err; + } + } val = DataType(val); } } } // Rebuild {property: {operator: value}} - if (operator) { + if (operator && operator !== 'eq') { var value = {}; value[operator] = val; if (exp.options) { diff --git a/test/allow-extended-operators.test.js b/test/allow-extended-operators.test.js index 359b73f3..19331dd3 100644 --- a/test/allow-extended-operators.test.js +++ b/test/allow-extended-operators.test.js @@ -67,50 +67,59 @@ describe('allowExtendedOperators', () => { } } + function assertOperatorNotAllowed(err) { + should.exist(err); + err.message.should.match(/Operators "\$exists" are not allowed in query/); + err.code.should.equal('OPERATOR_NOT_ALLOWED_IN_QUERY'); + err.statusCode.should.equal(400); + err.details.should.have.property('operators'); + err.details.should.have.property('where'); + } + describe('dataSource.settings.allowExtendedOperators', () => { context('DAO.find()', () => { - it('converts extended operators to string value by default', () => { + it('reports invalid operator by default', () => { const TestModel = createTestModel(); - return TestModel.find(extendedQuery()).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery()).catch(err => { + assertOperatorNotAllowed(err); }); }); it('preserves extended operators with allowExtendedOperators set', () => { const TestModel = createTestModel({allowExtendedOperators: true}); - return TestModel.find(extendedQuery()).then((results) => { + return TestModel.find(extendedQuery()).then(results => { should(results[0].value).eql({$exists: true}); }); }); it('`Model.settings.allowExtendedOperators` override data source settings - ' + - 'converts extended operators', () => { + 'reports invalid operator', () => { const TestModel = createTestModel({allowExtendedOperators: true}, {allowExtendedOperators: false}); - return TestModel.find(extendedQuery()).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery()).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`Model.settings.allowExtendedOperators` override data source settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}, {allowExtendedOperators: true}); - return TestModel.find(extendedQuery()).then((results) => { + return TestModel.find(extendedQuery()).then(results => { should(results[0].value).eql({$exists: true}); }); }); it('`options.allowExtendedOperators` override data source settings - ' + - 'converts extended operators', () => { + 'reports invalid operator', () => { const TestModel = createTestModel({allowExtendedOperators: true}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`options.allowExtendedOperators` override data source settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); @@ -168,37 +177,37 @@ describe('allowExtendedOperators', () => { context('DAO.find()', () => { it('preserves extended operators with allowExtendedOperators set', () => { const TestModel = createTestModel({}, {allowExtendedOperators: true}); - return TestModel.find(extendedQuery()).then((results) => { + return TestModel.find(extendedQuery()).then(results => { should(results[0].value).eql({$exists: true}); }); }); it('`dataSource.settings.allowExtendedOperators` honor Model settings - ' + - 'converts extended operators', () => { + 'reports invalid operator', () => { const TestModel = createTestModel({allowExtendedOperators: true}, {allowExtendedOperators: false}); - return TestModel.find(extendedQuery()).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery()).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`dataSource.settings.allowExtendedOperators` honor Model settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}, {allowExtendedOperators: true}); - return TestModel.find(extendedQuery()).then((results) => { + return TestModel.find(extendedQuery()).then(results => { should(results[0].value).eql({$exists: true}); }); }); it('`options.allowExtendedOperators` override Model settings - converts extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: true}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`options.allowExtendedOperators` Model settings - preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); @@ -255,7 +264,7 @@ describe('allowExtendedOperators', () => { context('DAO.find()', () => { it('preserves extended operators with allowExtendedOperators set', () => { const TestModel = createTestModel(); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); @@ -263,15 +272,15 @@ describe('allowExtendedOperators', () => { it('`dataSource.settings.allowExtendedOperators` honor options settings - ' + 'converts extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: true}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`dataSource.settings.allowExtendedOperators` honor options settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({allowExtendedOperators: false}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); @@ -279,15 +288,15 @@ describe('allowExtendedOperators', () => { it('`Model.settings.allowExtendedOperators` honor options settings - ' + 'converts extended operators', () => { const TestModel = createTestModel({}, {allowExtendedOperators: true}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).then((results) => { - should(results[0].value).eql('[object Object]'); + return TestModel.find(extendedQuery(), {allowExtendedOperators: false}).catch(err => { + assertOperatorNotAllowed(err); }); }); it('`Model.settings.allowExtendedOperators` honor options settings - ' + 'preserves extended operators', () => { const TestModel = createTestModel({}, {allowExtendedOperators: false}); - return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then((results) => { + return TestModel.find(extendedQuery(), {allowExtendedOperators: true}).then(results => { should(results[0].value).eql({$exists: true}); }); }); From 39ff54d39295fb4c620f561343156f56c2766aa1 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 18 Oct 2018 14:46:13 -0700 Subject: [PATCH 3/5] Hide offending properties from the error object --- lib/utils.js | 7 +++++-- test/model-definition.test.js | 3 +-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 9e67123e..88110069 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -32,6 +32,7 @@ exports.validateKeys = validateKeys; var g = require('strong-globalize')(); var traverse = require('traverse'); var assert = require('assert'); +var debug = require('debug')('loopback:juggler:utils'); function safeRequire(module) { try { @@ -362,11 +363,13 @@ function validateKeys(where, prohibitedKeys) { return x; }); if (offendingKeys.length) { - const msg = 'Properties "' + offendingKeys.join(', ') + '" are not allowed in query'; + const msg = 'Invalid properties are used in query'; const err = new Error(msg); err.code = 'PROPERTY_NOT_ALLOWED_IN_QUERY'; err.statusCode = 400; - err.details = {properties: offendingKeys, where: where}; + err.details = {where: where}; + debug('Hidden or protected properties %j are used in query: %j', + offendingKeys, where, err); throw err; } return result; diff --git a/test/model-definition.test.js b/test/model-definition.test.js index b0e025de..e11dfead 100644 --- a/test/model-definition.test.js +++ b/test/model-definition.test.js @@ -332,10 +332,9 @@ describe('ModelDefinition class', function() { function assertPropertyNotAllowed(err) { should.exist(err); - err.message.should.match(/Properties "secret" are not allowed in query/); + err.message.should.match(/Invalid properties are used in query/); err.code.should.equal('PROPERTY_NOT_ALLOWED_IN_QUERY'); err.statusCode.should.equal(400); - err.details.should.have.property('properties'); err.details.should.have.property('where'); } From 0ce3f4ead9914951aec275c1a4829d2bf5afb56d Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 19 Oct 2018 11:56:51 -0700 Subject: [PATCH 4/5] Report circular or deep query objects --- lib/dao.js | 64 +++++----- lib/include.js | 3 +- lib/include_utils.js | 4 +- lib/utils.js | 91 +++++++------ test/model-definition.test.js | 234 ++++++++++++++++++---------------- test/util.test.js | 54 ++++++-- 6 files changed, 254 insertions(+), 196 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index f119403b..c128b07b 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -26,7 +26,7 @@ var geo = require('./geo'); var Memory = require('./connectors/memory').Memory; var utils = require('./utils'); var fieldsToArray = utils.fieldsToArray; -var removeUndefined = utils.removeUndefined; +var sanitizeQueryOrData = utils.sanitizeQuery; var setScopeValuesFromWhere = utils.setScopeValuesFromWhere; var idEquals = utils.idEquals; var mergeQuery = utils.mergeQuery; @@ -406,7 +406,7 @@ DataAccessObject.create = function(data, options, cb) { obj.trigger('create', function(createDone) { obj.trigger('save', function(saveDone) { var _idName = idName(Model); - var val = removeUndefined(obj.toObject(true)); + var val = Model._sanitize(obj.toObject(true)); function createCallback(err, id, rev) { if (id) { obj.__data[_idName] = id; @@ -635,7 +635,7 @@ DataAccessObject.upsert = function(data, options, cb) { } function callConnector() { - update = removeUndefined(update); + update = Model._sanitize(update); context = { Model: Model, where: ctx.where, @@ -805,10 +805,10 @@ DataAccessObject.upsertWithWhere = function(where, data, options, cb) { try { // Support an optional where object var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - ctx.where = removeUndefined(ctx.where, handleUndefined); + // alter configuration of how sanitizeQuery handles undefined values + ctx.where = Model._sanitize(ctx.where, {handleUndefined: handleUndefined}); ctx.where = Model._coerce(ctx.where, options); - update = removeUndefined(update); + update = Model._sanitize(update); update = Model._coerce(update, options); } catch (err) { return process.nextTick(function() { @@ -989,7 +989,7 @@ DataAccessObject.replaceOrCreate = function replaceOrCreate(data, options, cb) { }, update, options); function callConnector() { - update = removeUndefined(update); + update = Model._sanitize(update); context = { Model: Model, where: where, @@ -1170,7 +1170,7 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb) }); } - data = removeUndefined(data); + data = Model._sanitize(data); var context = { Model: Model, where: query.where, @@ -1580,8 +1580,8 @@ DataAccessObject._normalize = function(filter, options) { } var handleUndefined = this._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - filter = removeUndefined(filter, handleUndefined); + // alter configuration of how sanitizeQuery handles undefined values + filter = this._sanitize(filter, {handleUndefined: handleUndefined}); this._coerce(filter.where, options); return filter; }; @@ -1664,6 +1664,18 @@ DataAccessObject._getProtectedProperties = function() { return result; }; +DataAccessObject._sanitize = function(query, options) { + options = options || {}; + + // Check violation of keys + var prohibitedKeys = this._getHiddenProperties(); + if (options.excludeProtectedProperties) { + prohibitedKeys = prohibitedKeys.concat(this._getProtectedProperties()); + } + return sanitizeQueryOrData(query, + Object.assign({prohibitedKeys: prohibitedKeys}, options)); +}; + /* * Coerce values based the property types * @param {Object} where The where clause @@ -1674,19 +1686,11 @@ DataAccessObject._getProtectedProperties = function() { */ DataAccessObject._coerce = function(where, options) { var self = this; - if (!where) { + if (where == null) { return where; } - options = options || {}; - // Check violation of keys - var prohibitedKeys = this._getHiddenProperties(); - if (options.excludeProtectedProperties) { - prohibitedKeys = prohibitedKeys.concat(this._getProtectedProperties()); - } - utils.validateKeys(where, prohibitedKeys); - var err; if (typeof where !== 'object' || Array.isArray(where)) { err = new Error(g.f('The where clause %j is not an {{object}}', where)); @@ -2322,8 +2326,8 @@ DataAccessObject.destroyAll = function destroyAll(where, options, cb) { try { // Support an optional where object var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - where = removeUndefined(where, handleUndefined); + // alter configuration of how sanitizeQuery handles undefined values + where = Model._sanitize(where, {handleUndefined: handleUndefined}); where = Model._coerce(where, options); } catch (err) { return process.nextTick(function() { @@ -2477,8 +2481,8 @@ DataAccessObject.count = function(where, options, cb) { try { var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - where = removeUndefined(where, handleUndefined); + // alter configuration of how sanitizeQuery handles undefined values + where = Model._sanitize(where, {handleUndefined: handleUndefined}); where = this._coerce(where, options); } catch (err) { process.nextTick(function() { @@ -2584,7 +2588,7 @@ DataAccessObject.prototype.save = function(options, cb) { function save() { inst.trigger('save', function(saveDone) { inst.trigger('update', function(updateDone) { - data = removeUndefined(data); + data = Model._sanitize(data); function saveCallback(err, unusedData, result) { if (err) { return cb(err, inst); @@ -2772,10 +2776,10 @@ DataAccessObject.updateAll = function(where, data, options, cb) { try { // Support an optional where object var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); - // alter configuration of how removeUndefined handles undefined values - where = removeUndefined(where, handleUndefined); + // alter configuration of how sanitizeQuery handles undefined values + where = Model._sanitize(where, {handleUndefined: handleUndefined}); where = Model._coerce(where, options); - data = removeUndefined(data); + data = Model._sanitize(data); data = Model._coerce(data, options); } catch (err) { return process.nextTick(function() { @@ -3108,7 +3112,7 @@ DataAccessObject.replaceById = function(id, data, options, cb) { function validateAndCallConnector(err, data) { if (err) return cb(err); - data = removeUndefined(data); + data = Model._sanitize(data); // update instance's properties inst.setAttributes(data); @@ -3260,7 +3264,7 @@ function(data, options, cb) { if (data instanceof Model) { data = data.toObject(false); } - data = removeUndefined(data); + data = Model._sanitize(data); // Make sure id(s) cannot be changed var idNames = Model.definition.idNames(); @@ -3333,7 +3337,7 @@ function(data, options, cb) { inst.trigger('update', function(done) { copyData(data, inst); var typedData = convertSubsetOfPropertiesByType(inst, data); - context.data = removeUndefined(typedData); + context.data = Model._sanitize(typedData); // Depending on the connector, the database response can // contain information about the updated record(s), but also diff --git a/lib/include.js b/lib/include.js index 18e94f37..04530236 100644 --- a/lib/include.js +++ b/lib/include.js @@ -205,7 +205,8 @@ Inclusion.include = function(objects, include, options, cb) { */ function findWithForeignKeysByPage(model, filter, fkName, pageSize, options, cb) { try { - model._coerce(filter.where, {excludeProtectedProperties: true}); + model._sanitize(filter.where, {excludeProtectedProperties: true}); + model._coerce(filter.where); } catch (e) { return cb(e); } diff --git a/lib/include_utils.js b/lib/include_utils.js index 70d70597..e08d722f 100644 --- a/lib/include_utils.js +++ b/lib/include_utils.js @@ -5,6 +5,8 @@ 'use strict'; +var g = require('strong-globalize')(); + module.exports.buildOneToOneIdentityMapWithOrigKeys = buildOneToOneIdentityMapWithOrigKeys; module.exports.buildOneToManyIdentityMapWithOrigKeys = buildOneToManyIdentityMapWithOrigKeys; module.exports.join = join; @@ -15,7 +17,7 @@ const util = require('util'); function getId(obj, idName) { var id = obj && obj[idName]; if (id == null) { - const msg = util.format('ID property "%s" is missing for included item: %j. ' + + const msg = g.f('ID property "%s" is missing for included item: %j. ' + 'Please make sure `fields` include "%s" if it\'s present in the `filter`', idName, obj, idName); const err = new Error(msg); diff --git a/lib/utils.js b/lib/utils.js index 88110069..2d1944b0 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -7,7 +7,7 @@ exports.safeRequire = safeRequire; exports.fieldsToArray = fieldsToArray; exports.selectFields = selectFields; -exports.removeUndefined = removeUndefined; +exports.sanitizeQuery = sanitizeQuery; exports.parseSettings = parseSettings; exports.mergeSettings = exports.deepMerge = deepMerge; exports.deepMergeProperty = deepMergeProperty; @@ -27,7 +27,6 @@ exports.collectTargetIds = collectTargetIds; exports.idName = idName; exports.rankArrayElements = rankArrayElements; exports.idsHaveDuplicates = idsHaveDuplicates; -exports.validateKeys = validateKeys; var g = require('strong-globalize')(); var traverse = require('traverse'); @@ -304,18 +303,53 @@ function selectFields(fields) { } /** - * Remove undefined values from the query object - * @param query - * @param handleUndefined {String} either "nullify", "throw" or "ignore" (default: "ignore") - * @returns {exports.map|*} + * Sanitize the query object + * @param query {object} The query object + * @param options + * @property handleUndefined {String} either "nullify", "throw" or "ignore" (default: "ignore") + * @property prohibitedKeys {String[]} An array of prohibited keys to be removed + * @returns {*} */ -function removeUndefined(query, handleUndefined) { +function sanitizeQuery(query, options) { + debug('Sanitizing query object: %j', query); if (typeof query !== 'object' || query === null) { return query; } + options = options || {}; + if (typeof options === 'string') { + // Keep it backward compatible + options = {handleUndefined: options}; + } + const prohibitedKeys = options.prohibitedKeys; + const offendingKeys = []; + const handleUndefined = options.handleUndefined; + const maxDepth = options.maxDepth || 10; // 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) { + const result = traverse(query).forEach(function(x) { + /** + * Security risk if the client passes in a very deep where object + */ + if (this.level > maxDepth || this.circular) { + const msg = g.f('The query object is too deep or circular'); + const err = new Error(msg); + err.statusCode = 400; + err.code = 'WHERE_OBJECT_TOO_DEEP'; + throw err; + } + /** + * Make sure prohibited keys are removed from the query to prevent + * sensitive values from being guessed + */ + if (prohibitedKeys && prohibitedKeys.indexOf(this.key) !== -1) { + offendingKeys.push(this.key); + this.remove(); + return; + } + + /** + * Handle undefined values + */ if (x === undefined) { switch (handleUndefined) { case 'nullify': @@ -323,7 +357,6 @@ function removeUndefined(query, handleUndefined) { break; case 'throw': throw new Error(g.f('Unexpected `undefined` in query')); - break; case 'ignore': default: this.remove(); @@ -339,44 +372,20 @@ function removeUndefined(query, handleUndefined) { return x; }); -} -/** - * Check the where clause to report prohibited keys - * @param {object} where Where object - * @param {string[]} [prohibitedKeys] An array of prohibited keys - */ -function validateKeys(where, prohibitedKeys) { - if (!prohibitedKeys || prohibitedKeys.length === 0) return where; - if (typeof where !== 'object' || where === null) { - return where; - } - prohibitedKeys = prohibitedKeys || []; - const offendingKeys = []; - // WARNING: [rfeng] Use traverse.map() will cause mongodb to produce invalid BSON - // as traverse doesn't transform the ObjectId correctly. Instead we have to use - // traverse(where).forEach(...) - const result = traverse(where).forEach(function(x) { - if (prohibitedKeys.indexOf(this.key) !== -1) { - offendingKeys.push(this.key); - } - return x; - }); if (offendingKeys.length) { - const msg = 'Invalid properties are used in query'; - const err = new Error(msg); - err.code = 'PROPERTY_NOT_ALLOWED_IN_QUERY'; - err.statusCode = 400; - err.details = {where: where}; - debug('Hidden or protected properties %j are used in query: %j', - offendingKeys, where, err); - throw err; + console.error( + g.f( + 'Potential security alert: hidden/protected properties %j are used in query.', + offendingKeys + ) + ); } return result; } -var url = require('url'); -var qs = require('qs'); +const url = require('url'); +const qs = require('qs'); /** * Parse a URL into a settings object diff --git a/test/model-definition.test.js b/test/model-definition.test.js index e11dfead..e8082962 100644 --- a/test/model-definition.test.js +++ b/test/model-definition.test.js @@ -277,11 +277,14 @@ describe('ModelDefinition class', function() { Parent.create({ name: 'parent', }, function(err, parent) { + if (err) return done(err); parent.children.create({ name: 'child', protectedProperty: 'protectedValue', }, function(err, child) { + if (err) return done(err); Parent.find({include: 'children'}, function(err, parents) { + if (err) return done(err); var serialized = parents[0].toJSON(); var child = serialized.children[0]; assert.equal(child.name, 'child'); @@ -315,11 +318,14 @@ describe('ModelDefinition class', function() { Parent.create({ name: 'parent', }, function(err, parent) { + if (err) return done(err); parent.children.create({ name: 'child', secret: 'secret', }, function(err, child) { + if (err) return done(err); Parent.find({include: 'children'}, function(err, parents) { + if (err) return done(err); var serialized = parents[0].toJSON(); var child = serialized.children[0]; assert.equal(child.name, 'child'); @@ -330,134 +336,138 @@ describe('ModelDefinition class', function() { }); }); - function assertPropertyNotAllowed(err) { - should.exist(err); - err.message.should.match(/Invalid properties are used in query/); - err.code.should.equal('PROPERTY_NOT_ALLOWED_IN_QUERY'); - err.statusCode.should.equal(400); - err.details.should.have.property('where'); + describe('hidden properties', function() { + var Child; + beforeEach(givenChildren); + + it('should be removed if used in where', function() { + return Child.find({ + where: {secret: 'guess'}, + }).then(assertHiddenPropertyIsIgnored); + }); + + it('should be removed if used in where.and', function() { + return Child.find({ + where: {and: [{secret: 'guess'}]}, + }).then(assertHiddenPropertyIsIgnored); + }); + + /** + * Create two children with a hidden property, one with a matching + * value, the other with a non-matching value + */ + function givenChildren() { + Child = memory.createModel('child', {}, {hidden: ['secret']}); + return Child.create([{ + name: 'childA', + secret: 'secret', + }, { + name: 'childB', + secret: 'guess', + }]); + } + + function assertHiddenPropertyIsIgnored(children) { + // All children are found whether the `secret` condition matches or not + // as the condition is removed because it's hidden + children.length.should.equal(2); + } + }); + + function assertParentIncludeChildren(parents) { + parents[0].toJSON().children.length.should.equal(1); } - it('should report errors if a hidden property is used in where', function(done) { - var Child = memory.createModel('child', {}, {hidden: ['secret']}); - Child.create({ - name: 'child', - secret: 'secret', - }, function(err) { - if (err) return done(err); - Child.find({ - where: {secret: 'secret'}, - }, function(err) { - assertPropertyNotAllowed(err); - done(); - }); - }); - }); + describe('protected properties', function() { + var Parent; + var Child; + beforeEach(givenParentAndChild); - it('should report errors if a hidden property is used in where.and', function(done) { - var Child = memory.createModel('child', {}, {hidden: ['secret']}); - Child.create({ - name: 'child', - secret: 'secret', - }, function(err) { - if (err) return done(err); - Child.find({ - where: {and: [{secret: 'secret'}]}, - }, function(err) { - assertPropertyNotAllowed(err); - done(); - }); - }); - }); - - it('should report errors if a protected property is used in include scope', function(done) { - var Parent = memory.createModel('parent'); - var Child = memory.createModel('child', {}, {protected: ['secret']}); - Parent.hasMany(Child); - Parent.create({ - name: 'parent', - }, function(err, parent) { - if (err) return done(err); - parent.children.create({ - name: 'child', - secret: 'secret', - }, function(err) { - if (err) return done(err); - Parent.find({ - include: { - relation: 'children', - scope: { - where: { - secret: 'x', - }, + it('should be removed if used in include scope', function() { + Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', }, }, - }, function(err) { - assertPropertyNotAllowed(err); - done(); - }); - }); + }, + }).then(assertParentIncludeChildren); }); - }); - it('should report errors if a protected property is used in include scope.where.and', function(done) { - var Parent = memory.createModel('parent'); - var Child = memory.createModel('child', {}, {protected: ['secret']}); - Parent.hasMany(Child); - Parent.create({ - name: 'parent', - }, function(err, parent) { - if (err) return done(err); - parent.children.create({ - name: 'child', - secret: 'secret', - }, function(err) { - if (err) return done(err); - Parent.find({ - include: { - relation: 'children', - scope: { - where: { - and: [{secret: 'x'}], - }, + it('should be rejected if used in include scope.where.and', function() { + return Parent.find({ + include: { + relation: 'children', + scope: { + where: { + and: [{secret: 'x'}], }, }, - }, function(err) { - assertPropertyNotAllowed(err); - done(); - }); - }); + }, + }).then(assertParentIncludeChildren); }); - }); - it('should report errors if a hidden property is used in include scope', function(done) { - var Parent = memory.createModel('parent'); - var Child = memory.createModel('child', {}, {hidden: ['secret']}); - Parent.hasMany(Child); - Parent.create({ - name: 'parent', - }, function(err, parent) { - if (err) return done(err); - parent.children.create({ - name: 'child', - secret: 'secret', - }, function(err) { - if (err) return done(err); - Parent.find({ - include: { - relation: 'children', - scope: { - where: { - secret: 'x', - }, + it('should be removed if a hidden property is used in include scope', function() { + return Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', }, }, - }, function(err) { - assertPropertyNotAllowed(err); - done(); + }, + }).then(assertParentIncludeChildren); + }); + + function givenParentAndChild() { + Parent = memory.createModel('parent'); + Child = memory.createModel('child', {}, {protected: ['secret']}); + Parent.hasMany(Child); + return Parent.create({ + name: 'parent', + }).then(parent => { + return parent.children.create({ + name: 'child', + secret: 'secret', }); }); + } + }); + + describe('hidden properties in include', function() { + var Parent; + var Child; + beforeEach(givenParentAndChildWithHiddenProperty); + + it('should be rejected if used in scope', function() { + return Parent.find({ + include: { + relation: 'children', + scope: { + where: { + secret: 'x', + }, + }, + }, + }).then(assertParentIncludeChildren); }); + + function givenParentAndChildWithHiddenProperty() { + Parent = memory.createModel('parent'); + Child = memory.createModel('child', {}, {hidden: ['secret']}); + Parent.hasMany(Child); + return Parent.create({ + name: 'parent', + }).then(parent => { + return parent.children.create({ + name: 'child', + secret: 'secret', + }); + }); + } }); it('should throw error for property names containing dot', function() { diff --git a/test/util.test.js b/test/util.test.js index 132bdf64..81593432 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -8,7 +8,7 @@ var should = require('./init.js'); var utils = require('../lib/utils'); var ObjectID = require('bson').ObjectID; var fieldsToArray = utils.fieldsToArray; -var removeUndefined = utils.removeUndefined; +var sanitizeQuery = utils.sanitizeQuery; var deepMerge = utils.deepMerge; var rankArrayElements = utils.rankArrayElements; var mergeIncludes = utils.mergeIncludes; @@ -52,33 +52,65 @@ describe('util.fieldsToArray', function() { }); }); -describe('util.removeUndefined', function() { +describe('util.sanitizeQuery', function() { it('Remove undefined values from the query object', function() { var q1 = {where: {x: 1, y: undefined}}; - should.deepEqual(removeUndefined(q1), {where: {x: 1}}); + should.deepEqual(sanitizeQuery(q1), {where: {x: 1}}); var q2 = {where: {x: 1, y: 2}}; - should.deepEqual(removeUndefined(q2), {where: {x: 1, y: 2}}); + should.deepEqual(sanitizeQuery(q2), {where: {x: 1, y: 2}}); var q3 = {where: {x: 1, y: {in: [2, undefined]}}}; - should.deepEqual(removeUndefined(q3), {where: {x: 1, y: {in: [2]}}}); + should.deepEqual(sanitizeQuery(q3), {where: {x: 1, y: {in: [2]}}}); - should.equal(removeUndefined(null), null); + should.equal(sanitizeQuery(null), null); - should.equal(removeUndefined(undefined), undefined); + should.equal(sanitizeQuery(undefined), undefined); - should.equal(removeUndefined('x'), 'x'); + should.equal(sanitizeQuery('x'), 'x'); var date = new Date(); var q4 = {where: {x: 1, y: date}}; - should.deepEqual(removeUndefined(q4), {where: {x: 1, y: date}}); + should.deepEqual(sanitizeQuery(q4), {where: {x: 1, y: date}}); // test handling of undefined var q5 = {where: {x: 1, y: undefined}}; - should.deepEqual(removeUndefined(q5, 'nullify'), {where: {x: 1, y: null}}); + should.deepEqual(sanitizeQuery(q5, 'nullify'), {where: {x: 1, y: null}}); + + q5 = {where: {x: 1, y: undefined}}; + should.deepEqual(sanitizeQuery(q5, {handleUndefined: 'nullify'}), {where: {x: 1, y: null}}); var q6 = {where: {x: 1, y: undefined}}; - (function() { removeUndefined(q6, 'throw'); }).should.throw(/`undefined` in query/); + (function() { sanitizeQuery(q6, 'throw'); }).should.throw(/`undefined` in query/); + }); + + it('Report errors for circular or deep query objects', function() { + var q7 = {where: {x: 1}}; + q7.where.y = q7; + (function() { sanitizeQuery(q7); }).should.throw( + /The query object is too deep or circular/ + ); + + var q8 = {where: {and: [{and: [{and: [{and: [{and: [{and: + [{and: [{and: [{and: [{x: 1}]}]}]}]}]}]}]}]}]}}; + (function() { sanitizeQuery(q8); }).should.throw( + /The query object is too deep or circular/ + ); + + var q9 = {where: {and: [{and: [{and: [{and: [{x: 1}]}]}]}]}}; + (function() { sanitizeQuery(q8, {maxDepth: 4}); }).should.throw( + /The query object is too deep or circular/ + ); + }); + + it('Removed prohibited properties in query objects', function() { + var q1 = {where: {secret: 'guess'}}; + sanitizeQuery(q1, {prohibitedKeys: ['secret']}); + q1.where.should.eql({}); + + var q2 = {and: [{secret: 'guess'}, {x: 1}]}; + sanitizeQuery(q2, {prohibitedKeys: ['secret']}); + q2.should.eql({and: [{}, {x: 1}]}); }); }); From a39176277103277b44c69540476efcabddd915d6 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 26 Oct 2018 09:05:47 -0700 Subject: [PATCH 5/5] Ren handleUndefined to normalizeUndefinedInQuery --- lib/dao.js | 20 ++++++++++---------- lib/utils.js | 8 ++++---- test/util.test.js | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index c128b07b..96d0692a 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -804,9 +804,9 @@ DataAccessObject.upsertWithWhere = function(where, data, options, cb) { function callConnector() { try { // Support an optional where object - var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); + var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery'); // alter configuration of how sanitizeQuery handles undefined values - ctx.where = Model._sanitize(ctx.where, {handleUndefined: handleUndefined}); + ctx.where = Model._sanitize(ctx.where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery}); ctx.where = Model._coerce(ctx.where, options); update = Model._sanitize(update); update = Model._coerce(update, options); @@ -1579,9 +1579,9 @@ DataAccessObject._normalize = function(filter, options) { Object.keys(this.definition.properties), this.settings.strict); } - var handleUndefined = this._getSetting('normalizeUndefinedInQuery'); + var normalizeUndefinedInQuery = this._getSetting('normalizeUndefinedInQuery'); // alter configuration of how sanitizeQuery handles undefined values - filter = this._sanitize(filter, {handleUndefined: handleUndefined}); + filter = this._sanitize(filter, {normalizeUndefinedInQuery: normalizeUndefinedInQuery}); this._coerce(filter.where, options); return filter; }; @@ -2325,9 +2325,9 @@ DataAccessObject.destroyAll = function destroyAll(where, options, cb) { } else { try { // Support an optional where object - var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); + var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery'); // alter configuration of how sanitizeQuery handles undefined values - where = Model._sanitize(where, {handleUndefined: handleUndefined}); + where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery}); where = Model._coerce(where, options); } catch (err) { return process.nextTick(function() { @@ -2480,9 +2480,9 @@ DataAccessObject.count = function(where, options, cb) { where = query.where; try { - var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); + var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery'); // alter configuration of how sanitizeQuery handles undefined values - where = Model._sanitize(where, {handleUndefined: handleUndefined}); + where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery}); where = this._coerce(where, options); } catch (err) { process.nextTick(function() { @@ -2775,9 +2775,9 @@ DataAccessObject.updateAll = function(where, data, options, cb) { function doUpdate(where, data) { try { // Support an optional where object - var handleUndefined = Model._getSetting('normalizeUndefinedInQuery'); + var normalizeUndefinedInQuery = Model._getSetting('normalizeUndefinedInQuery'); // alter configuration of how sanitizeQuery handles undefined values - where = Model._sanitize(where, {handleUndefined: handleUndefined}); + where = Model._sanitize(where, {normalizeUndefinedInQuery: normalizeUndefinedInQuery}); where = Model._coerce(where, options); data = Model._sanitize(data); data = Model._coerce(data, options); diff --git a/lib/utils.js b/lib/utils.js index 2d1944b0..7620dba8 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -306,7 +306,7 @@ function selectFields(fields) { * Sanitize the query object * @param query {object} The query object * @param options - * @property handleUndefined {String} either "nullify", "throw" or "ignore" (default: "ignore") + * @property normalizeUndefinedInQuery {String} either "nullify", "throw" or "ignore" (default: "ignore") * @property prohibitedKeys {String[]} An array of prohibited keys to be removed * @returns {*} */ @@ -318,11 +318,11 @@ function sanitizeQuery(query, options) { options = options || {}; if (typeof options === 'string') { // Keep it backward compatible - options = {handleUndefined: options}; + options = {normalizeUndefinedInQuery: options}; } const prohibitedKeys = options.prohibitedKeys; const offendingKeys = []; - const handleUndefined = options.handleUndefined; + const normalizeUndefinedInQuery = options.normalizeUndefinedInQuery; const maxDepth = options.maxDepth || 10; // WARNING: [rfeng] Use map() will cause mongodb to produce invalid BSON // as traverse doesn't transform the ObjectId correctly @@ -351,7 +351,7 @@ function sanitizeQuery(query, options) { * Handle undefined values */ if (x === undefined) { - switch (handleUndefined) { + switch (normalizeUndefinedInQuery) { case 'nullify': this.update(null); break; diff --git a/test/util.test.js b/test/util.test.js index 81593432..3874faeb 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -78,7 +78,7 @@ describe('util.sanitizeQuery', function() { should.deepEqual(sanitizeQuery(q5, 'nullify'), {where: {x: 1, y: null}}); q5 = {where: {x: 1, y: undefined}}; - should.deepEqual(sanitizeQuery(q5, {handleUndefined: 'nullify'}), {where: {x: 1, y: null}}); + should.deepEqual(sanitizeQuery(q5, {normalizeUndefinedInQuery: 'nullify'}), {where: {x: 1, y: null}}); var q6 = {where: {x: 1, y: undefined}}; (function() { sanitizeQuery(q6, 'throw'); }).should.throw(/`undefined` in query/);