diff options
author | Spencer Jackson <spencer.jackson@mongodb.com> | 2016-12-19 18:17:04 -0500 |
---|---|---|
committer | Spencer Jackson <spencer.jackson@mongodb.com> | 2017-05-03 16:34:54 -0400 |
commit | 74a3cb1bc1faee3d1aae2b207e41502fb5c98150 (patch) | |
tree | a700fc47ce08750aa50b87d18d5e9ae74c2fd494 /src/mongo | |
parent | f55883b2c7e530a669e9c93ae3a41654ab4dae4f (diff) | |
download | mongo-74a3cb1bc1faee3d1aae2b207e41502fb5c98150.tar.gz |
SERVER-27570: Enforce stricter checks on top level command BSON objects
(cherry picked from commit 957549cd11a34e0c92c9996eb564a6e2d6ada58a)
Diffstat (limited to 'src/mongo')
-rw-r--r-- | src/mongo/bson/SConscript | 13 | ||||
-rw-r--r-- | src/mongo/bson/ugly_bson_integration_test.cpp | 63 | ||||
-rw-r--r-- | src/mongo/db/commands/dbcommands.cpp | 54 | ||||
-rw-r--r-- | src/mongo/s/s_only.cpp | 23 |
4 files changed, 120 insertions, 33 deletions
diff --git a/src/mongo/bson/SConscript b/src/mongo/bson/SConscript index d2f20e3fc1c..f983418ea01 100644 --- a/src/mongo/bson/SConscript +++ b/src/mongo/bson/SConscript @@ -68,3 +68,16 @@ env.CppUnitTest( '$BUILD_DIR/mongo/base', ], ) + +asioEnv = env.Clone() +asioEnv.InjectThirdPartyIncludePaths('asio') + +asioEnv.CppIntegrationTest( + target='ugly_bson_integration_test', + source=[ + 'ugly_bson_integration_test.cpp' + ], + LIBDEPS=[ + '$BUILD_DIR/mongo/executor/network_interface_asio_fixture', + ], +) diff --git a/src/mongo/bson/ugly_bson_integration_test.cpp b/src/mongo/bson/ugly_bson_integration_test.cpp new file mode 100644 index 00000000000..aec92665824 --- /dev/null +++ b/src/mongo/bson/ugly_bson_integration_test.cpp @@ -0,0 +1,63 @@ +/** + * Copyright (C) 2017 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 <algorithm> +#include <exception> + +#include "mongo/client/connection_string.h" +#include "mongo/executor/network_interface_asio_integration_fixture.h" +#include "mongo/unittest/integration_test.h" +#include "mongo/unittest/unittest.h" +#include "mongo/util/assert_util.h" + +namespace mongo { +namespace executor { +namespace { + +class UglyBSONFixture : public NetworkInterfaceASIOIntegrationFixture { + void setUp() override { + startNet(); + } +}; + +TEST_F(UglyBSONFixture, DuplicateFields) { + assertCommandFailsOnServer("admin", + BSON("insert" + << "test" + << "documents" + << BSONArray() + << "documents" + << BSONArray()), + ErrorCodes::FailedToParse); +} + +} // namespace +} // namespace executor +} // namespace mongo diff --git a/src/mongo/db/commands/dbcommands.cpp b/src/mongo/db/commands/dbcommands.cpp index d577aa5dde2..c282579a7f1 100644 --- a/src/mongo/db/commands/dbcommands.cpp +++ b/src/mongo/db/commands/dbcommands.cpp @@ -35,6 +35,7 @@ #include <time.h> #include "mongo/base/disallow_copying.h" +#include "mongo/base/simple_string_data_comparator.h" #include "mongo/base/status.h" #include "mongo/base/status_with.h" #include "mongo/bson/simple_bsonobj_comparator.h" @@ -1222,22 +1223,6 @@ private: bool maintenanceModeSet; }; -namespace { - -// Symbolic names for indexes to make code more readable. -const std::size_t kCmdOptionMaxTimeMSField = 0; -const std::size_t kHelpField = 1; -const std::size_t kShardVersionFieldIdx = 2; -const std::size_t kQueryOptionMaxTimeMSField = 3; - -// We make an array of the fields we need so we can call getFields once. This saves repeated -// scans over the command object. -const std::array<StringData, 4> neededFieldNames{QueryRequest::cmdOptionMaxTimeMS, - Command::kHelpFieldName, - ChunkVersion::kShardVersionField, - QueryRequest::queryOptionMaxTimeMS}; -} // namespace - void appendOpTimeMetadata(OperationContext* txn, const rpc::RequestInterface& request, BSONObjBuilder* metadataBob) { @@ -1296,11 +1281,31 @@ void Command::execCommand(OperationContext* txn, std::string dbname = request.getDatabase().toString(); unique_ptr<MaintenanceModeSetter> mmSetter; - std::array<BSONElement, std::tuple_size<decltype(neededFieldNames)>::value> - extractedFields{}; - request.getCommandArgs().getFields(neededFieldNames, &extractedFields); + BSONElement cmdOptionMaxTimeMSField; + BSONElement helpField; + BSONElement shardVersionFieldIdx; + BSONElement queryOptionMaxTimeMSField; + + StringMap<int> topLevelFields; + for (auto&& element : request.getCommandArgs()) { + StringData fieldName = element.fieldNameStringData(); + if (fieldName == QueryRequest::cmdOptionMaxTimeMS) { + cmdOptionMaxTimeMSField = element; + } else if (fieldName == Command::kHelpFieldName) { + helpField = element; + } else if (fieldName == ChunkVersion::kShardVersionField) { + shardVersionFieldIdx = element; + } else if (fieldName == QueryRequest::queryOptionMaxTimeMS) { + queryOptionMaxTimeMSField = element; + } + + uassert(ErrorCodes::FailedToParse, + str::stream() << "Parsed command object contains duplicate top level key: " + << fieldName, + topLevelFields[fieldName]++ == 0); + } - if (isHelpRequest(extractedFields[kHelpField])) { + if (Command::isHelpRequest(helpField)) { CurOp::get(txn)->ensureStarted(); // We disable last-error for help requests due to SERVER-11492, because config servers // use help requests to determine which commands are database writes, and so must be @@ -1367,12 +1372,11 @@ void Command::execCommand(OperationContext* txn, } // Handle command option maxTimeMS. - int maxTimeMS = uassertStatusOK( - QueryRequest::parseMaxTimeMS(extractedFields[kCmdOptionMaxTimeMSField])); + int maxTimeMS = uassertStatusOK(QueryRequest::parseMaxTimeMS(cmdOptionMaxTimeMSField)); uassert(ErrorCodes::InvalidOptions, "no such command option $maxTimeMs; use maxTimeMS instead", - extractedFields[kQueryOptionMaxTimeMSField].eoo()); + queryOptionMaxTimeMSField.eoo()); if (maxTimeMS > 0) { uassert(40119, @@ -1389,10 +1393,8 @@ void Command::execCommand(OperationContext* txn, auto& oss = OperationShardingState::get(txn); auto commandNS = NamespaceString(command->parseNs(dbname, request.getCommandArgs())); - oss.initializeShardVersion(commandNS, extractedFields[kShardVersionFieldIdx]); - + oss.initializeShardVersion(commandNS, shardVersionFieldIdx); auto shardingState = ShardingState::get(txn); - if (oss.hasShardVersion()) { if (serverGlobalParams.clusterRole != ClusterRole::ShardServer) { uassertStatusOK( diff --git a/src/mongo/s/s_only.cpp b/src/mongo/s/s_only.cpp index 2fcfc4c30b2..f8f9365994e 100644 --- a/src/mongo/s/s_only.cpp +++ b/src/mongo/s/s_only.cpp @@ -96,13 +96,22 @@ void Command::execCommandClient(OperationContext* txn, BSONObjBuilder& result) { std::string dbname = nsToDatabase(ns); - if (cmdObj.getBoolField("help")) { - stringstream help; - help << "help for: " << c->getName() << " "; - c->help(help); - result.append("help", help.str()); - appendCommandStatus(result, true, ""); - return; + StringMap<int> topLevelFields; + for (auto&& element : cmdObj) { + StringData fieldName = element.fieldNameStringData(); + if (fieldName == "help" && element.type() == Bool && element.Bool()) { + std::stringstream help; + help << "help for: " << c->getName() << " "; + c->help(help); + result.append("help", help.str()); + Command::appendCommandStatus(result, true, ""); + return; + } + + uassert(ErrorCodes::FailedToParse, + str::stream() << "Parsed command object contains duplicate top level key: " + << fieldName, + topLevelFields[fieldName]++ == 0); } Status status = checkAuthorization(c, txn, dbname, cmdObj); |