Skip to content

dbviewer: harden aggregation and query handling #7690

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.

Sign up for GitHub

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

Closed
ar2rsawseen wants to merge 1 commit into from
+734 −86
Closed

dbviewer: harden aggregation and query handling #7690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
185 changes: 99 additions & 86 deletions plugins/dbviewer/api/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,83 +12,21 @@ const { MongoInvalidArgumentError } = require('mongodb');
const { EJSON } = require('bson');

const FEATURE_NAME = 'dbviewer';
const whiteListedAggregationStages = {
"$addFields": true,
"$bucket": true,
"$bucketAuto": true,
//"$changeStream": false,
//"$changeStreamSplitLargeEvents": false,
//"$collStats": false,
"$count": true,
//"$currentOp": false,
"$densify": true,
//"$documents": false
"$facet": true,
"$fill": true,
"$geoNear": true,
// "$graphLookup": false — removed: lets attacker pull joined documents from any collection in the same DB,
// bypassing the per-collection access check. Use $lookup instead if cross-collection
// joins are ever needed (currently also disallowed).
"$group": true,
//"$indexStats": false,
"$limit": true,
//"$listLocalSessions": false
//"$listSampledQueries": false
//"$listSearchIndexes": false
//"$listSessions": false
//"$lookup": false
"$match": true,
//"$merge": false
//"$mergeCursors": false
//"$out": false
//"$planCacheStats": false,
"$project": true,
"$querySettings": true,
"$redact": true,
"$replaceRoot": true,
"$replaceWith": true,
"$sample": true,
"$search": true,
"$searchMeta": true,
"$set": true,
"$setWindowFields": true,
//"$sharedDataDistribution": false,
"$skip": true,
"$sort": true,
"$sortByCount": true,
//"$unionWith": false,
"$unset": true,
"$unwind": true,
"$vectorSearch": true //atlas specific
};
// upper bound on rows returned per find()/aggregation page, to keep a crafted
// limit/iDisplayLength from requesting an unbounded result set
const MAX_DBVIEWER_LIMIT = 10000;
// Aggregation-stage allow-list and the recursive sanitizer that strips blocked
// stages at every depth (including inside $facet sub-pipelines). Kept in a
// dedicated module so it can be unit-tested in isolation.
const { escapeNotAllowedAggregationStages, findProtectedCollectionJoin, findWriteStage, findServerSideJs } = require('./parts/aggregation_guard.js');
const { sanitizeProjection, escapeRegExp } = require('./parts/query_guard.js');
var spawn = require('child_process').spawn,
child;

(function() {
plugins.register("/permissions/features", function(ob) {
ob.features.push(FEATURE_NAME);
});
/**
* Function removes not allowed aggregation stages from the pipeline
* @param {array} aggregation - current aggregation pipeline
* @returns {object} changes - object with information which operations were removed
*/
function escapeNotAllowedAggregationStages(aggregation) {
var changes = {};
for (var z = 0; z < aggregation.length; z++) {
for (var key in aggregation[z]) {
if (!whiteListedAggregationStages[key]) {
changes[key] = true;
delete aggregation[z][key];
}
}
if (Object.keys(aggregation[z]).length === 0) {
aggregation.splice(z, 1);
z--;
}
}
return changes;
}

/**
* @api {get} /o/db Access database
Expand Down Expand Up @@ -189,11 +127,22 @@ var spawn = require('child_process').spawn,
params.qstring.document = common.db.ObjectID(params.qstring.document);
}
if (dbs[dbNameOnParam]) {
dbs[dbNameOnParam].collection(params.qstring.collection).findOne({ _id: params.qstring.document }, function(err, results) {
// Scope the lookup to the member's apps the same way the
// collection listing does, so a document cannot be fetched
// outside the caller's app scope by supplying its _id.
var docFilter = { _id: params.qstring.document };
if (!params.member.global_admin) {
var docBaseFilter = getBaseAppFilter(params.member, dbNameOnParam, params.qstring.collection);
if (docBaseFilter && Object.keys(docBaseFilter).length > 0) {
docFilter = { $and: [docBaseFilter, docFilter] };
}
}
dbs[dbNameOnParam].collection(params.qstring.collection).findOne(docFilter, function(err, results) {
if (!err) {
if (params.qstring.collection === 'members' && results) {
delete results.password;
delete results.api_key;
delete results.two_factor_auth;
}
else if (params.qstring.collection === 'auth_tokens' && results) {
if (results._id) {
Expand All @@ -203,7 +152,8 @@ var spawn = require('child_process').spawn,
common.returnOutput(params, objectIdCheck(results) || {});
}
else {
common.returnOutput(params, 500, err);
log.e(err);
common.returnMessage(params, 500, "An unexpected error occurred.");
}
});
}
Expand All @@ -216,8 +166,19 @@ var spawn = require('child_process').spawn,
* Get collection data from db
**/
async function dbGetCollection() {
var limit = parseInt(params.qstring.limit || 20);
var skip = parseInt(params.qstring.skip || 0);
// cap page size and guard against NaN so a crafted limit/skip can't
// request an unbounded result set
var limit = parseInt(params.qstring.limit, 10);
if (isNaN(limit) || limit <= 0) {
limit = 20;
}
if (limit > MAX_DBVIEWER_LIMIT) {
limit = MAX_DBVIEWER_LIMIT;
}
var skip = parseInt(params.qstring.skip, 10);
if (isNaN(skip) || skip < 0) {
skip = 0;
}
var filter = params.qstring.filter || params.qstring.query || "{}";
var sSearch = params.qstring.sSearch || "";
var projection = params.qstring.project || params.qstring.projection || "{}";
Expand Down Expand Up @@ -248,14 +209,22 @@ var spawn = require('child_process').spawn,
filter._id = common.db.ObjectID(filter._id);
}
if (sSearch) {
filter._id = new RegExp(sSearch);
// treat the search term as a literal so a crafted pattern cannot
// cause catastrophic regex backtracking (ReDoS)
filter._id = new RegExp(escapeRegExp(sSearch));
}
try {
projection = EJSON.parse(projection);
}
catch (SyntaxError) {
projection = {};
}
//EJSON.parse("null") yields null and an array is also typeof
//"object"; normalize anything that isn't a plain object to {} so an
//invalid projection can't reach find()
if (!projection || typeof projection !== 'object' || Array.isArray(projection)) {
projection = {};
}
if (typeof filter !== 'object' || Array.isArray(filter)) {
filter = {};
}
Expand All @@ -265,6 +234,10 @@ var spawn = require('child_process').spawn,
//viewer query cannot be abused to execute code on the server
common.stripUnsafeMongoOperators(filter);
common.stripUnsafeMongoOperators(sort);
//restrict the projection to plain field include/exclude — drop any
//expression / field-path alias (e.g. {x:"$password"}) that could
//compute or rename fields the viewer otherwise removes
sanitizeProjection(projection);

var base_filter = {};
if (!params.member.global_admin) {
Expand Down Expand Up @@ -310,6 +283,7 @@ var spawn = require('child_process').spawn,
if (params.qstring.collection === 'members' && doc) {
delete doc.password;
delete doc.api_key;
delete doc.two_factor_auth;
}
else if (params.qstring.collection === 'auth_tokens' && doc) {
if (doc._id) {
Expand Down Expand Up @@ -357,7 +331,8 @@ var spawn = require('child_process').spawn,
}
}
catch (err) {
common.returnMessage(params, 500, err);
log.e(err);
common.returnMessage(params, 500, "An unexpected error occurred.");
}
}
}
Expand Down Expand Up @@ -429,23 +404,25 @@ var spawn = require('child_process').spawn,
* */
function aggregate(collection, aggregation, changes) {
if (params.qstring.iDisplayLength) {
aggregation.push({ "$limit": parseInt(params.qstring.iDisplayLength) });
var iDisplayLength = parseInt(params.qstring.iDisplayLength, 10);
if (!isNaN(iDisplayLength) && iDisplayLength > 0) {
aggregation.push({ "$limit": Math.min(iDisplayLength, MAX_DBVIEWER_LIMIT) });
Comment on lines 405 to +409
}
}
if (!Array.isArray(aggregation)) {
common.returnMessage(params, 500, "The aggregation pipeline must be of the type array");
}
else {
var addProjectionAt = 0;
if (aggregation[0] && aggregation[0].$match) {
while (aggregation.length > addProjectionAt && aggregation[addProjectionAt].$match) {
addProjectionAt++;
}
}
if (collection === 'members') {
aggregation.splice(addProjectionAt, 0, {"$project": {"password": 0, "api_key": 0}});
// Insert the redaction as the very first stage so no
// user-supplied stage — including a leading $match using
// $expr, or a $project/$group that aliases or references the
// field — can read the raw credential fields before they are
// removed.
aggregation.splice(0, 0, {"$project": {"password": 0, "api_key": 0, "two_factor_auth": 0}});
}
else if (collection === 'auth_tokens') {
aggregation.splice(addProjectionAt, 0, {"$addFields": {"_id": "***redacted***"}});
aggregation.splice(0, 0, {"$addFields": {"_id": "***redacted***"}});
}
else if ((collection === "events_data" || collection === "drill_events") && !params.member.global_admin) {
var base_filter = getBaseAppFilter(params.member, dbNameOnParam, params.qstring.collection);
Expand Down Expand Up @@ -490,7 +467,8 @@ var spawn = require('child_process').spawn,
common.returnOutput(params, { sEcho: params.qstring.sEcho, iTotalRecords: 0, iTotalDisplayRecords: 0, "aaData": result, "removed": (changes || {}) });
}
else {
common.returnMessage(params, 500, aggregationErr);
log.e(aggregationErr);
common.returnMessage(params, 500, "An unexpected error occurred.");
}
}
});
Expand Down Expand Up @@ -593,6 +571,26 @@ var spawn = require('child_process').spawn,
if (params.member.global_admin) {
try {
let aggregation = EJSON.parse(params.qstring.aggregation);
// A join into a redacted collection (members / auth_tokens)
// would return raw credentials, since the redaction only
// applies to the top-level source collection. This is
// blocked even for global admins, who are intentionally
// denied raw api_key / password / tokens via DB Viewer.
var jsOp = findServerSideJs(aggregation);
if (jsOp) {
common.returnMessage(params, 400, 'Aggregation may not use the "' + jsOp + '" operator');
return true;
}
var writeStage = findWriteStage(aggregation);
if (writeStage) {
common.returnMessage(params, 400, 'Aggregation may not use the "' + writeStage + '" stage');
return true;
}
var protectedJoin = findProtectedCollectionJoin(aggregation);
if (protectedJoin) {
common.returnMessage(params, 400, 'Aggregation may not join the "' + protectedJoin + '" collection');
return true;
}
aggregate(params.qstring.collection, aggregation);
}
catch (e) {
Expand All @@ -605,6 +603,21 @@ var spawn = require('child_process').spawn,
if (hasAccess || params.qstring.collection === "events_data" || params.qstring.collection === "drill_events") {
try {
let aggregation = EJSON.parse(params.qstring.aggregation);
var jsOpRef = findServerSideJs(aggregation);
if (jsOpRef) {
common.returnMessage(params, 400, 'Aggregation may not use the "' + jsOpRef + '" operator');
return true;
}
var writeStageRef = findWriteStage(aggregation);
if (writeStageRef) {
common.returnMessage(params, 400, 'Aggregation may not use the "' + writeStageRef + '" stage');
return true;
}
var protectedJoinRef = findProtectedCollectionJoin(aggregation);
if (protectedJoinRef) {
common.returnMessage(params, 400, 'Aggregation may not join the "' + protectedJoinRef + '" collection');
return true;
}
var changes = escapeNotAllowedAggregationStages(aggregation);
if (changes && Object.keys(changes).length > 0) {
log.d("Removed stages from pipeline: ", JSON.stringify(changes));
Expand Down
Loading
Loading