From c5b7b711fecdc2ca855c7bdb0742945564c557b7 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 25 Jun 2024 12:43:27 -0400 Subject: [PATCH 1/2] feat(query): make sanitizeProjection prevent projecting in paths deselected in the schema --- lib/query.js | 12 ++++++++++- lib/queryHelpers.js | 10 ++++++++-- test/query.test.js | 41 ++++++++++++++++++++++++++++++++++++++ test/schema.select.test.js | 13 ++++++++++++ 4 files changed, 73 insertions(+), 3 deletions(-) diff --git a/lib/query.js b/lib/query.js index 5bb3ee9a611..4cd3466415b 100644 --- a/lib/query.js +++ b/lib/query.js @@ -4866,7 +4866,17 @@ Query.prototype._applyPaths = function applyPaths() { return; } this._fields = this._fields || {}; - helpers.applyPaths(this._fields, this.model.schema); + + let sanitizeProjection = undefined; + if (this.model != null && utils.hasUserDefinedProperty(this.model.db.options, 'sanitizeProjection')) { + sanitizeProjection = this.model.db.options.sanitizeProjection; + } else if (this.model != null && utils.hasUserDefinedProperty(this.model.base.options, 'sanitizeProjection')) { + sanitizeProjection = this.model.base.options.sanitizeProjection; + } else { + sanitizeProjection = this._mongooseOptions.sanitizeProjection; + } + + helpers.applyPaths(this._fields, this.model.schema, sanitizeProjection); let _selectPopulatedPaths = true; diff --git a/lib/queryHelpers.js b/lib/queryHelpers.js index 90d8739ef8b..9431095199e 100644 --- a/lib/queryHelpers.js +++ b/lib/queryHelpers.js @@ -145,7 +145,7 @@ exports.createModelAndInit = function createModelAndInit(model, doc, fields, use * ignore */ -exports.applyPaths = function applyPaths(fields, schema) { +exports.applyPaths = function applyPaths(fields, schema, sanitizeProjection) { // determine if query is selecting or excluding fields let exclude; let keys; @@ -321,6 +321,10 @@ exports.applyPaths = function applyPaths(fields, schema) { // User overwriting default exclusion if (type.selected === false && fields[path]) { + if (sanitizeProjection) { + fields[path] = 0; + } + return; } @@ -345,8 +349,10 @@ exports.applyPaths = function applyPaths(fields, schema) { // if there are other fields being included, add this one // if no other included fields, leave this out (implied inclusion) - if (exclude === false && keys.length > 1 && !~keys.indexOf(path)) { + if (exclude === false && keys.length > 1 && !~keys.indexOf(path) && !sanitizeProjection) { fields[path] = 1; + } else if (exclude == null && sanitizeProjection && type.selected === false) { + fields[path] = 0; } return; diff --git a/test/query.test.js b/test/query.test.js index 1073bb089e6..553ab04e152 100644 --- a/test/query.test.js +++ b/test/query.test.js @@ -3449,6 +3449,47 @@ describe('Query', function() { assert.deepEqual(q._fields, { email: 1 }); }); + it('sanitizeProjection option with plus paths (gh-14333) (gh-10243)', async function() { + const MySchema = Schema({ + name: String, + email: String, + password: { type: String, select: false } + }); + const Test = db.model('Test', MySchema); + + await Test.create({ name: 'test', password: 'secret' }); + + let q = Test.findOne().select('+password'); + let doc = await q; + assert.deepEqual(q._fields, {}); + assert.strictEqual(doc.password, 'secret'); + + q = Test.findOne().setOptions({ sanitizeProjection: true }).select('+password'); + doc = await q; + assert.deepEqual(q._fields, { password: 0 }); + assert.strictEqual(doc.password, undefined); + + q = Test.find().select('+password').setOptions({ sanitizeProjection: true }); + doc = await q; + assert.deepEqual(q._fields, { password: 0 }); + assert.strictEqual(doc.password, undefined); + + q = Test.find().select('name +password').setOptions({ sanitizeProjection: true }); + doc = await q; + assert.deepEqual(q._fields, { name: 1 }); + assert.strictEqual(doc.password, undefined); + + q = Test.find().select('+name').setOptions({ sanitizeProjection: true }); + doc = await q; + assert.deepEqual(q._fields, { password: 0 }); + assert.strictEqual(doc.password, undefined); + + q = Test.find().select('password').setOptions({ sanitizeProjection: true }); + doc = await q; + assert.deepEqual(q._fields, { password: 0 }); + assert.strictEqual(doc.password, undefined); + }); + it('sanitizeFilter option (gh-3944)', function() { const MySchema = Schema({ username: String, pwd: String }); const Test = db.model('Test', MySchema); diff --git a/test/schema.select.test.js b/test/schema.select.test.js index b9e6806d90d..e34af328327 100644 --- a/test/schema.select.test.js +++ b/test/schema.select.test.js @@ -346,6 +346,19 @@ describe('schema select option', function() { assert.equal(d.id, doc.id); }); + it('works if only one plus path and only one deselected field', async function() { + const MySchema = Schema({ + name: String, + email: String, + password: { type: String, select: false } + }); + const Test = db.model('Test', MySchema); + const { _id } = await Test.create({ name: 'test', password: 'secret' }); + + const doc = await Test.findById(_id).select('+password'); + assert.strictEqual(doc.password, 'secret'); + }); + it('works with query.slice (gh-1370)', async function() { const M = db.model('Test', new Schema({ many: { type: [String], select: false } })); From 0904a18c740d7ba893744cc4f40449e8f97e1bb5 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 1 Jul 2024 14:00:12 -0400 Subject: [PATCH 2/2] feat(query): add sanitizeProjection method to query for better docs --- lib/query.js | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ types/query.d.ts | 5 +++++ 2 files changed, 58 insertions(+) diff --git a/lib/query.js b/lib/query.js index 9eff42a9317..ccca65f4192 100644 --- a/lib/query.js +++ b/lib/query.js @@ -1142,6 +1142,59 @@ Query.prototype.select = function select() { throw new TypeError('Invalid select() argument. Must be string or object.'); }; +/** + * Sets this query's `sanitizeProjection` option. If set, `sanitizeProjection` does + * two things: + * + * 1. Enforces that projection values are numbers, not strings. + * 2. Prevents using `+` syntax to override properties that are deselected by default. + * + * With `sanitizeProjection()`, you can pass potentially untrusted user data to `.select()`. + * + * #### Example + * + * const userSchema = new Schema({ + * name: String, + * password: { type: String, select: false } + * }); + * const UserModel = mongoose.model('User', userSchema); + * const { _id } = await UserModel.create({ name: 'John', password: 'secret' }) + * + * // The MongoDB server has special handling for string values that start with '$' + * // in projections, which can lead to unexpected leaking of sensitive data. + * let doc = await UserModel.findOne().select({ name: '$password' }); + * doc.name; // 'secret' + * doc.password; // undefined + * + * // With `sanitizeProjection`, Mongoose forces all projection values to be numbers + * doc = await UserModel.findOne().sanitizeProjection(true).select({ name: '$password' }); + * doc.name; // 'John' + * doc.password; // undefined + * + * // By default, Mongoose supports projecting in `password` using `+password` + * doc = await UserModel.findOne().select('+password'); + * doc.password; // 'secret' + * + * // With `sanitizeProjection`, Mongoose prevents projecting in `password` and other + * // fields that have `select: false` in the schema. + * doc = await UserModel.findOne().sanitizeProjection(true).select('+password'); + * doc.password; // undefined + * + * @method sanitizeProjection + * @memberOf Query + * @instance + * @param {Boolean} value + * @return {Query} this + * @see sanitizeProjection https://thecodebarbarian.com/whats-new-in-mongoose-5-13-sanitizeprojection.html + * @api public + */ + +Query.prototype.sanitizeProjection = function sanitizeProjection(value) { + this._mongooseOptions.sanitizeProjection = value; + + return this; +}; + /** * Determines the MongoDB nodes from which to read. * diff --git a/types/query.d.ts b/types/query.d.ts index 6b6fb6b8cd1..44d9a71062f 100644 --- a/types/query.d.ts +++ b/types/query.d.ts @@ -716,6 +716,11 @@ declare module 'mongoose' { options?: QueryOptions | null ): QueryWithHelpers; + /** + * Sets this query's `sanitizeProjection` option. With `sanitizeProjection()`, you can pass potentially untrusted user data to `.select()`. + */ + sanitizeProjection(value: boolean): this; + /** Specifies which document fields to include or exclude (also known as the query "projection") */ select( arg: string | string[] | Record