SERVER-41829 findAndModify ignores filter expressions that are not objects

3 files changed, 60 insertions, 6 deletions

diff --git a/src/mongo/db/query/find_and_modify_request.cpp b/src/mongo/db/query/find_and_modify_request.cpp
index f98e8a00eb3..20f62d2a407 100644
--- a/src/mongo/db/query/find_and_modify_request.cpp
+++ b/src/mongo/db/query/find_and_modify_request.cpp
@@ -168,9 +168,23 @@ StatusWith<FindAndModifyRequest> FindAndModifyRequest::parseFromBSON(NamespaceSt
 
     for (auto&& field : cmdObj.getFieldNames<std::set<std::string>>()) {
         if (field == kQueryField) {
-            query = cmdObj.getObjectField(kQueryField);
+            auto queryElement = cmdObj[kQueryField];
+            if (queryElement.type() != Object) {
+                return {ErrorCodes::Error(31160),
+                        str::stream() << "'" << kQueryField
+                                      << "' parameter must be an object, found "
+                                      << queryElement.type()};
+            }
+            query = queryElement.embeddedObject();
         } else if (field == kSortField) {
-            sort = cmdObj.getObjectField(kSortField);
+            auto sortElement = cmdObj[kSortField];
+            if (sortElement.type() != Object) {
+                return {ErrorCodes::Error(31174),
+                        str::stream() << "'" << kSortField
+                                      << "' parameter must be an object, found "
+                                      << sortElement.type()};
+            }
+            sort = sortElement.embeddedObject();
         } else if (field == kRemoveField) {
             isRemove = cmdObj[kRemoveField].trueValue();
         } else if (field == kUpdateField) {
@@ -178,7 +192,14 @@ StatusWith<FindAndModifyRequest> FindAndModifyRequest::parseFromBSON(NamespaceSt
         } else if (field == kNewField) {
             shouldReturnNew = cmdObj[kNewField].trueValue();
         } else if (field == kFieldProjectionField) {
-            fields = cmdObj.getObjectField(kFieldProjectionField);
+            auto projectionElement = cmdObj[kFieldProjectionField];
+            if (projectionElement.type() != Object) {
+                return {ErrorCodes::Error(31175),
+                        str::stream() << "'" << kFieldProjectionField
+                                      << "' parameter must be an object, found "
+                                      << projectionElement.type()};
+            }
+            fields = projectionElement.embeddedObject();
         } else if (field == kUpsertField) {
             isUpsert = cmdObj[kUpsertField].trueValue();
         } else if (field == kBypassDocumentValidationField) {
diff --git a/src/mongo/db/query/find_and_modify_request_test.cpp b/src/mongo/db/query/find_and_modify_request_test.cpp
index 29f0fc4ac60..c60590a2a01 100644
--- a/src/mongo/db/query/find_and_modify_request_test.cpp
+++ b/src/mongo/db/query/find_and_modify_request_test.cpp
@@ -649,5 +649,38 @@ TEST(FindAndModifyRequest, RejectsBothArrayFiltersAndPipelineUpdate) {
     auto swRequestOneFilter = FindAndModifyRequest::parseFromBSON(NamespaceString("a.b"), cmdObj);
     ASSERT_EQ(swRequestOneFilter.getStatus(), ErrorCodes::FailedToParse);
 }
+
+TEST(FindAndModifyRequest, InvalidQueryParameter) {
+    BSONObj cmdObj(fromjson(R"json({
+            findAndModify: 'user',
+            query: '{ x: 1 }',
+            remove: true
+        })json"));
+
+    auto parseStatus = FindAndModifyRequest::parseFromBSON(NamespaceString("a.b"), cmdObj);
+    ASSERT_EQ(31160, parseStatus.getStatus().code());
+}
+
+TEST(FindAndModifyRequest, InvalidSortParameter) {
+    BSONObj cmdObj(fromjson(R"json({
+            findAndModify: 'user',
+            sort: 1,
+            remove: true
+        })json"));
+
+    auto parseStatus = FindAndModifyRequest::parseFromBSON(NamespaceString("a.b"), cmdObj);
+    ASSERT_EQ(31174, parseStatus.getStatus().code());
+}
+
+TEST(FindAndModifyRequest, InvalidFieldParameter) {
+    BSONObj cmdObj(fromjson(R"json({
+            findAndModify: 'user',
+            fields: null,
+            remove: true
+        })json"));
+
+    auto parseStatus = FindAndModifyRequest::parseFromBSON(NamespaceString("a.b"), cmdObj);
+    ASSERT_EQ(31175, parseStatus.getStatus().code());
+}
 }  // unnamed namespace
 }  // namespace mongo
diff --git a/src/mongo/shell/crud_api.js b/src/mongo/shell/crud_api.js
index ffed81f9c15..bcd245f4878 100644
--- a/src/mongo/shell/crud_api.js
+++ b/src/mongo/shell/crud_api.js
@@ -708,7 +708,7 @@ DBCollection.prototype.updateMany = function(filter, update, options) {
 DBCollection.prototype.findOneAndDelete = function(filter, options) {
     var opts = Object.extend({}, options || {});
     // Set up the command
-    var cmd = {query: filter, remove: true};
+    var cmd = {query: filter || {}, remove: true};
 
     if (opts.sort) {
         cmd.sort = opts.sort;
@@ -771,7 +771,7 @@ DBCollection.prototype.findOneAndReplace = function(filter, replacement, options
     }
 
     // Set up the command
-    var cmd = {query: filter, update: replacement};
+    var cmd = {query: filter || {}, update: replacement};
     if (opts.sort) {
         cmd.sort = opts.sort;
     }
@@ -839,7 +839,7 @@ DBCollection.prototype.findOneAndUpdate = function(filter, update, options) {
     }
 
     // Set up the command
-    var cmd = {query: filter, update: update};
+    var cmd = {query: filter || {}, update: update};
     if (opts.sort) {
         cmd.sort = opts.sort;
     }