SERVER-43287 Prevent from outputting to system collections

6 files changed, 91 insertions, 18 deletions

diff --git a/jstests/aggregation/bugs/server3253.js b/jstests/aggregation/bugs/server3253.js
index 5df6736e88f..352eb11ed02 100644
--- a/jstests/aggregation/bugs/server3253.js
+++ b/jstests/aggregation/bugs/server3253.js
@@ -17,10 +17,6 @@ input.drop();
 inputDoesntExist.drop();  // never created
 output.drop();
 
-function collectionExists(coll) {
-    return Array.contains(coll.getDB().getCollectionNames(), coll.getName());
-}
-
 function getOutputIndexes() {
     return output.getIndexes().sort(function(a, b) {
         if (a.name < b.name) {
@@ -104,10 +100,5 @@ if (jsTest.options().storageEngine !== "mobile") {
 // ensure everything works even if input doesn't exist.
 test(inputDoesntExist, [], []);
 
-// ensure we cant do dangerous things to system collections
-var outputInSystem = db.system.server3253_out;
-assertErrorCode(input, {$out: outputInSystem.getName()}, 17385);
-assert(!collectionExists(outputInSystem));
-
 // shoudn't leave temp collections laying around
 assert.eq([], listCollections(/tmp\.agg_out/));
diff --git a/jstests/aggregation/extras/utils.js b/jstests/aggregation/extras/utils.js
index 7364ecde54d..7afef1ec657 100644
--- a/jstests/aggregation/extras/utils.js
+++ b/jstests/aggregation/extras/utils.js
@@ -357,3 +357,10 @@ function generateCollection({
     assert.eq(numDocs, res.nInserted);
     assert.eq(numDocs, coll.find().itcount());
 }
+
+/**
+ * Returns true if 'coll' exists or false otherwise.
+ */
+function collectionExists(coll) {
+    return Array.contains(coll.getDB().getCollectionNames(), coll.getName());
+}
diff --git a/jstests/aggregation/no_output_to_system.js b/jstests/aggregation/no_output_to_system.js
new file mode 100644
index 00000000000..8a68a29926b
--- /dev/null
+++ b/jstests/aggregation/no_output_to_system.js
@@ -0,0 +1,63 @@
+// Tests that you cannot use aggregation to output to a system collection.
+(function() {
+"use strict";
+
+load('jstests/aggregation/extras/utils.js');  // For 'assertErrorCode'.
+load("jstests/libs/fixture_helpers.js");      // For 'isMongos'.
+
+const input = db.no_output_to_system;
+input.drop();
+assert.commandWorked(input.insert({_id: 0}));
+
+// Ensure that aggregation can't output to system collection.
+const outputInSystem = db.system.no_output_to_system;
+assertErrorCode(input, {$out: outputInSystem.getName()}, 17385);
+assert(!collectionExists(outputInSystem));
+
+assertErrorCode(input, {$merge: outputInSystem.getName()}, 31319);
+assert(!collectionExists(outputInSystem));
+
+// Ensure that aggregation can't output to the 'admin' database.
+const admin = db.getSiblingDB('admin');
+const outputToAdmin = admin.users;
+
+assertErrorCode(input, {$merge: {into: {db: 'admin', coll: outputToAdmin.getName()}}}, 31320);
+assert(!collectionExists(outputToAdmin));
+
+// We specify a separate input since $out must reference a collection in the same database.
+const adminInput = admin.sample;
+adminInput.drop();
+assert.commandWorked(adminInput.insert({_id: 0}));
+
+assertErrorCode(adminInput, {$out: outputToAdmin.getName()}, 31321);
+assert(!collectionExists(outputToAdmin));
+
+// Ensure that aggregation can't output to the 'config' database.
+const config = db.getSiblingDB('config');
+const outputToConfig = config.output;
+
+assertErrorCode(input, {$merge: {into: {db: 'config', coll: outputToConfig.getName()}}}, 31320);
+assert(!collectionExists(outputToConfig));
+
+// We specify a separate input since $out must reference a collection in the same database.
+const configInput = config.sample;
+configInput.drop();
+assert.commandWorked(configInput.insert({_id: 0}));
+
+assertErrorCode(configInput, {$out: outputToConfig.getName()}, 31321);
+assert(!collectionExists(outputToConfig));
+
+// Ensure that aggregation can't output to the 'local' database.
+// Only test if local exists (i.e. we are not on mongos).
+if (!FixtureHelpers.isMongos(db)) {
+    const local = db.getSiblingDB('local');
+
+    // Every mongod has this collection.
+    const outputToLocal = local.startup_log;
+
+    assertErrorCode(input, {$merge: {into: {db: 'local', coll: outputToLocal.getName()}}}, 31320);
+
+    // $out allows for the source collection to be the same as the destination collection.
+    assertErrorCode(outputToLocal, {$out: outputToLocal.getName()}, 31321);
+}
+})();
\ No newline at end of file
diff --git a/jstests/sharding/agg_write_stages_cannot_run_on_mongos.js b/jstests/sharding/agg_write_stages_cannot_run_on_mongos.js
index 05a48adf3eb..d677410499f 100644
--- a/jstests/sharding/agg_write_stages_cannot_run_on_mongos.js
+++ b/jstests/sharding/agg_write_stages_cannot_run_on_mongos.js
@@ -5,7 +5,6 @@
 
 const st = new ShardingTest({shards: 2, rs: {nodes: 1}, config: 1});
 const db = st.s0.getDB("db");
-const admin = st.s0.getDB("admin");
 
 // Create a collection in the db to get around optimizations that will do nothing in lieu of
 // failing when the db is empty.
@@ -16,21 +15,22 @@ assert.commandWorked(db.runCommand({create: "coll"}));
 assert.commandFailedWithCode(
     db.runCommand({aggregate: 1, pipeline: [{$listLocalSessions: {}}, {$out: "test"}], cursor: {}}),
     ErrorCodes.IllegalOperation);
-assert.commandFailedWithCode(
-    admin.runCommand(
-        {aggregate: 1, pipeline: [{$currentOp: {localOps: true}}, {$out: "test"}], cursor: {}}),
-    ErrorCodes.IllegalOperation);
+assert.commandFailedWithCode(db.runCommand({
+    aggregate: "coll",
+    pipeline: [{$_internalSplitPipeline: {mergeType: "mongos"}}, {$out: "test"}],
+    cursor: {}
+}),
+                             ErrorCodes.IllegalOperation);
 assert.commandFailedWithCode(
     db.runCommand({aggregate: 1, pipeline: [{$changeStream: {}}, {$out: "test"}], cursor: {}}),
     ErrorCodes.IllegalOperation);
-
 assert.commandFailedWithCode(
     db.runCommand(
         {aggregate: 1, pipeline: [{$listLocalSessions: {}}, {$merge: {into: "test"}}], cursor: {}}),
     ErrorCodes.IllegalOperation);
-assert.commandFailedWithCode(admin.runCommand({
-    aggregate: 1,
-    pipeline: [{$currentOp: {localOps: true}}, {$merge: {into: "test"}}],
+assert.commandFailedWithCode(db.runCommand({
+    aggregate: "coll",
+    pipeline: [{$_internalSplitPipeline: {mergeType: "mongos"}}, {$merge: {into: "test"}}],
     cursor: {}
 }),
                              ErrorCodes.IllegalOperation);
diff --git a/src/mongo/db/pipeline/document_source_merge.cpp b/src/mongo/db/pipeline/document_source_merge.cpp
index 119219b9e7b..a804dc921f9 100644
--- a/src/mongo/db/pipeline/document_source_merge.cpp
+++ b/src/mongo/db/pipeline/document_source_merge.cpp
@@ -377,6 +377,14 @@ boost::intrusive_ptr<DocumentSource> DocumentSourceMerge::create(
             "{} cannot be used with a 'linearizable' read concern level"_format(kStageName),
             readConcernLevel != repl::ReadConcernLevel::kLinearizableReadConcern);
 
+    uassert(31319,
+            "Cannot {} to special collection: {}"_format(kStageName, outputNs.coll()),
+            !outputNs.isSystem());
+
+    uassert(31320,
+            "Cannot {} to internal database: {}"_format(kStageName, outputNs.db()),
+            !outputNs.isOnInternalDb());
+
     if (whenMatched == WhenMatched::kPipeline) {
         if (!letVariables) {
             // For custom pipeline-style updates, default the 'let' variables to {new: "$$ROOT"},
diff --git a/src/mongo/db/pipeline/document_source_out.cpp b/src/mongo/db/pipeline/document_source_out.cpp
index 1208bcea38b..860d9028bec 100644
--- a/src/mongo/db/pipeline/document_source_out.cpp
+++ b/src/mongo/db/pipeline/document_source_out.cpp
@@ -199,6 +199,10 @@ boost::intrusive_ptr<DocumentSource> DocumentSourceOut::create(
             "Can't {} to special collection: {}"_format(kStageName, outputNs.coll()),
             !outputNs.isSystem());
 
+    uassert(31321,
+            "Can't {} to internal database: {}"_format(kStageName, outputNs.db()),
+            !outputNs.isOnInternalDb());
+
     return new DocumentSourceOut(std::move(outputNs), expCtx);
 }