diff --git a/api/utils/common.js b/api/utils/common.js index 05119c59521..ca7e0c01915 100644 --- a/api/utils/common.js +++ b/api/utils/common.js @@ -3075,6 +3075,40 @@ common.checkDatabaseConfigMatch = (apiConfig, frontendConfig) => { } }; +/** +* Restrict a find() projection to plain field inclusion and exclusion. +* +* A projection value may only be 0, 1 or a boolean. Anything else is dropped: +* - expressions and field path aliases, for example { leak: "$password" } or +* { x: { $function: ... } }, would rename or compute fields that the caller is not +* supposed to see. MongoDB 4.4 and later accept aggregation expressions in a find() +* projection, so a rename defeats any redaction that works by field name, and +* $function evaluates javascript in the database engine; +* - other numbers such as 2 or NaN are not valid include/exclude values and can make the +* query throw. +* +* Kept in common so every caller supplied projection goes through the same guard, whether +* it arrives at the DB Viewer or at an export. +* @param {object} projection - parsed projection object, mutated in place +* @returns {object} changes - keys are the projection fields that were dropped +*/ +common.sanitizeProjection = function(projection) { + var changes = {}; + if (!projection || typeof projection !== "object" || Array.isArray(projection)) { + return changes; + } + for (var key in projection) { + if (Object.prototype.hasOwnProperty.call(projection, key)) { + var value = projection[key]; + if (value !== 0 && value !== 1 && value !== true && value !== false) { + changes[key] = true; + delete projection[key]; + } + } + } + return changes; +}; + common.sanitizeFilename = (filename, replacement = "") => { return (filename + "") .replace(/[\x00-\x1f\x80-\x9f]+/g, replacement) diff --git a/api/utils/requestProcessor.js b/api/utils/requestProcessor.js index 3287fe1c7dc..b7543b3864e 100644 --- a/api/utils/requestProcessor.js +++ b/api/utils/requestProcessor.js @@ -2152,6 +2152,12 @@ const processRequest = (params) => { params.qstring.projection = null; } } + //The projection reaches find() as given, and the credential redaction + //further down works by field name, so an expression that renames or + //computes a field would carry a redacted value out under a name the + //redaction does not know. Restrict it to plain include and exclude, the + //same guard the DB Viewer applies to its own projections. + common.sanitizeProjection(params.qstring.projection); if (typeof params.qstring.sort === "string") { try { params.qstring.sort = JSON.parse(params.qstring.sort); diff --git a/plugins/dbviewer/api/parts/query_guard.js b/plugins/dbviewer/api/parts/query_guard.js index db0bbd85dd8..df7e60f4898 100644 --- a/plugins/dbviewer/api/parts/query_guard.js +++ b/plugins/dbviewer/api/parts/query_guard.js @@ -6,6 +6,8 @@ 'use strict'; +const common = require('../../../../api/utils/common.js'); + /** * Restrict a find() projection to plain field inclusion / exclusion. * @@ -23,20 +25,8 @@ * @returns {object} changes - keys are the projection fields that were dropped */ function sanitizeProjection(projection) { - var changes = {}; - if (!projection || typeof projection !== "object" || Array.isArray(projection)) { - return changes; - } - for (var key in projection) { - if (Object.prototype.hasOwnProperty.call(projection, key)) { - var value = projection[key]; - if (value !== 0 && value !== 1 && value !== true && value !== false) { - changes[key] = true; - delete projection[key]; - } - } - } - return changes; + //one implementation, in common, shared with the export paths + return common.sanitizeProjection(projection); } /** diff --git a/test/unit-tests/api.utils.common.js b/test/unit-tests/api.utils.common.js index b68bcc2a74b..e3fbb684221 100644 --- a/test/unit-tests/api.utils.common.js +++ b/test/unit-tests/api.utils.common.js @@ -504,3 +504,51 @@ describe("Common API utility functions", function() { }); }); }); + +// The export path and the DB Viewer both hand a caller supplied projection to find(), and +// MongoDB 4.4 and later evaluate aggregation expressions there. An expression can rename a +// field, which defeats redaction that works by field name, or run javascript in the database +// with $function. Only plain include and exclude survive. +describe('common.sanitizeProjection', function() { + it('keeps plain include and exclude', function() { + var p = {email: 1, full_name: 1, password: 0, api_key: false, name: true}; + common.sanitizeProjection(p); + p.should.eql({email: 1, full_name: 1, password: 0, api_key: false, name: true}); + }); + + it('drops a field path alias, which is how a redacted field gets renamed', function() { + var p = {pw: '$password', ak: '$api_key', tfa: '$two_factor_auth', email: 1}; + var dropped = common.sanitizeProjection(p); + p.should.eql({email: 1}); + Object.keys(dropped).sort().should.eql(['ak', 'pw', 'tfa']); + }); + + it('drops a nested field path alias', function() { + var p = {secret: '$two_factor_auth.secret_token'}; + common.sanitizeProjection(p); + p.should.eql({}); + }); + + it('drops expressions, including ones that would run javascript', function() { + var p = { + joined: {$concat: ['$password', '$api_key']}, + js: {$function: {body: 'function(v){ return v; }', args: ['$password'], lang: 'js'}}, + picked: {$cond: [true, '$api_key', 'no']} + }; + common.sanitizeProjection(p); + p.should.eql({}); + }); + + it('drops values that are not valid include or exclude', function() { + var p = {a: 2, b: NaN, c: 'yes', d: null, e: [1]}; + common.sanitizeProjection(p); + p.should.eql({}); + }); + + it('leaves a missing or non object projection alone', function() { + should.not.exist(common.sanitizeProjection(undefined).undefined); + Object.keys(common.sanitizeProjection(null)).should.eql([]); + Object.keys(common.sanitizeProjection('string')).should.eql([]); + Object.keys(common.sanitizeProjection([1, 2])).should.eql([]); + }); +});