SERVER-19028 validate database name in request class constructors

7 files changed, 82 insertions, 11 deletions

diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index f999084fdd7..e3f88f80b28 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -245,13 +245,8 @@ namespace {
         rpc::LegacyReplyBuilder builder{};
 
         try {
+            // This will throw if the request is on an invalid namespace.
             rpc::LegacyRequest request{&message};
-            // Do the namespace validity check under the try/catch block so it does not cause the
-            // connection to be terminated.
-            uassert(ErrorCodes::InvalidNamespace,
-                    str::stream() << "Invalid ns [" << request.getDatabase() << ".$cmd" << "]",
-                    NamespaceString::validDBName(request.getDatabase()));
-
             // Auth checking for Commands happens later.
             int nToReturn = queryMessage.ntoreturn;
             beginQueryOp(txn, nss, queryMessage.query, nToReturn, queryMessage.ntoskip);
diff --git a/src/mongo/rpc/SConscript b/src/mongo/rpc/SConscript
index d70a0f7d194..d541d1163fb 100644
--- a/src/mongo/rpc/SConscript
+++ b/src/mongo/rpc/SConscript
@@ -143,12 +143,13 @@ env.CppUnitTest(
         'rpc_test',
     ],
     source=[
+        'command_reply_builder_test.cpp',
         'command_reply_test.cpp',
         'command_request_builder_test.cpp',
         'command_request_test.cpp',
+        'legacy_request_test.cpp',
         'object_check_test.cpp',
         'protocol_test.cpp',
-        'command_reply_builder_test.cpp',
     ],
     LIBDEPS=[
         'rpc',
diff --git a/src/mongo/rpc/command_request.cpp b/src/mongo/rpc/command_request.cpp
index a6c73eeb389..d5fd8fa4515 100644
--- a/src/mongo/rpc/command_request.cpp
+++ b/src/mongo/rpc/command_request.cpp
@@ -38,6 +38,7 @@
 #include "mongo/base/data_type_terminated.h"
 #include "mongo/base/data_type_validated.h"
 #include "mongo/db/jsobj.h"
+#include "mongo/db/namespace_string.h"
 #include "mongo/rpc/object_check.h"
 #include "mongo/util/assert_util.h"
 #include "mongo/util/mongoutils/str.h"
@@ -76,6 +77,10 @@ namespace rpc {
                 (_database.size() >= kMinDatabaseLength) &&
                 (_database.size() <= kMaxDatabaseLength));
 
+        uassert(ErrorCodes::InvalidNamespace,
+                str::stream() << "Invalid database name: '" << _database << "'",
+                NamespaceString::validDBName(_database));
+
         _commandName = uassertStatusOK(cur.readAndAdvance<Terminated<'\0', StringData>>());
 
         uassert(28637, str::stream() << "Command name parsed in OP_COMMAND message must be between"
diff --git a/src/mongo/rpc/command_request_test.cpp b/src/mongo/rpc/command_request_test.cpp
index 7f5c5b52214..64732735d6f 100644
--- a/src/mongo/rpc/command_request_test.cpp
+++ b/src/mongo/rpc/command_request_test.cpp
@@ -34,14 +34,16 @@
 
 #include "mongo/db/jsobj.h"
 #include "mongo/rpc/command_request.h"
+#include "mongo/rpc/command_request_builder.h"
 #include "mongo/unittest/unittest.h"
 #include "mongo/util/net/message.h"
+#include "mongo/util/assert_util.h"
 
 namespace {
 
     using namespace mongo;
 
-    TEST(RequestTest, ParseAllFields) {
+    TEST(CommandRequest, ParseAllFields) {
         std::vector<char> opCommandData;
 
         using std::begin;
@@ -106,4 +108,15 @@ namespace {
 
         ASSERT_TRUE(inputDocRangeIter == inputDocRange.end());
     }
+
+    TEST(CommandRequest, InvalidNSThrows) {
+        rpc::CommandRequestBuilder crb;
+        crb.setDatabase("foo////!!!!<><><>");
+        crb.setCommandName("foo");
+        crb.setMetadata(BSONObj());
+        crb.setCommandArgs(BSON("ping" << 1));
+        auto msg = crb.done();
+        ASSERT_THROWS(rpc::CommandRequest{msg.get()}, AssertionException);
+    }
+
 }  // namespace
diff --git a/src/mongo/rpc/legacy_request.cpp b/src/mongo/rpc/legacy_request.cpp
index e4f09257257..57f3f12cb58 100644
--- a/src/mongo/rpc/legacy_request.cpp
+++ b/src/mongo/rpc/legacy_request.cpp
@@ -41,8 +41,13 @@ namespace rpc {
     LegacyRequest::LegacyRequest(const Message *message)
         : _message(std::move(message))
         , _dbMessage(*message)
-        , _queryMessage(_dbMessage)
-        , _database(nsToDatabase(_queryMessage.ns)) {
+        , _queryMessage(_dbMessage) {
+
+        _database = nsToDatabaseSubstring(_queryMessage.ns);
+
+        uassert(ErrorCodes::InvalidNamespace,
+                str::stream() << "Invalid database name: '" << _database << "'",
+                NamespaceString::validDBName(_database));
 
         std::tie(_upconvertedCommandArgs, _upconvertedMetadata) = uassertStatusOK(
             rpc::upconvertRequestMetadata(std::move(_queryMessage.query),
diff --git a/src/mongo/rpc/legacy_request.h b/src/mongo/rpc/legacy_request.h
index eb6df488e26..59371600894 100644
--- a/src/mongo/rpc/legacy_request.h
+++ b/src/mongo/rpc/legacy_request.h
@@ -96,7 +96,7 @@ namespace rpc {
         // for now getMetadata() is a no op
         DbMessage _dbMessage;
         QueryMessage _queryMessage;
-        std::string _database;
+        StringData _database;
 
         BSONObj _upconvertedMetadata;
         BSONObj _upconvertedCommandArgs;
diff --git a/src/mongo/rpc/legacy_request_test.cpp b/src/mongo/rpc/legacy_request_test.cpp
new file mode 100644
index 00000000000..a24b8eba072
--- /dev/null
+++ b/src/mongo/rpc/legacy_request_test.cpp
@@ -0,0 +1,52 @@
+/**
+ * Copyright (C) 2015 MongoDB Inc.
+ *
+ * This program is free software: you can redistribute it and/or  modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * As a special exception, the copyright holders give permission to link the
+ * code of portions of this program with the OpenSSL library under certain
+ * conditions as described in each individual source file and distribute
+ * linked combinations including the program with the OpenSSL library. You
+ * must comply with the GNU Affero General Public License in all respects
+ * for all of the code used other than as permitted herein. If you modify
+ * file(s) with this exception, you may extend this exception to your
+ * version of the file(s), but you are not obligated to do so. If you do not
+ * wish to do so, delete this exception statement from your version. If you
+ * delete this exception statement from all source files in the program,
+ * then also delete it in the license file.
+ */
+
+#include "mongo/platform/basic.h"
+
+#include "mongo/db/jsobj.h"
+#include "mongo/rpc/legacy_request.h"
+#include "mongo/rpc/legacy_request_builder.h"
+
+#include "mongo/unittest/unittest.h"
+#include "mongo/util/assert_util.h"
+
+namespace {
+
+    using namespace mongo;
+
+    TEST(LegacyRequest, InvalidNSThrows) {
+        rpc::LegacyRequestBuilder crb;
+        crb.setDatabase("foo////!!!!<><><>");
+        crb.setCommandName("foo");
+        crb.setMetadata(BSONObj());
+        crb.setCommandArgs(BSON("ping" << 1));
+        auto msg = crb.done();
+        ASSERT_THROWS(rpc::LegacyRequest{msg.get()}, AssertionException);
+    }
+
+}  // namespace