Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(query): make sanitizeProjection prevent projecting in paths deselected in the schema #14691

Merged
merged 3 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 64 additions & 1 deletion lib/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -4872,7 +4925,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;

Expand Down
10 changes: 8 additions & 2 deletions lib/queryHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down
41 changes: 41 additions & 0 deletions test/query.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 13 additions & 0 deletions test/schema.select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 } }));

Expand Down
5 changes: 5 additions & 0 deletions types/query.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,11 @@ declare module 'mongoose' {
options?: QueryOptions<DocType> | null
): QueryWithHelpers<any, DocType, THelpers, RawDocType, 'replaceOne', TInstanceMethods>;

/**
* 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<RawDocTypeOverride extends { [P in keyof RawDocType]?: any } = {}>(
arg: string | string[] | Record<string, number | boolean | string | object>
Expand Down