-
Type:
Bug
-
Resolution: Fixed
-
Priority:
Major - P3
-
Affects Version/s: None
-
Component/s: None
-
Labels:
-
Fully Compatible
-
Service arch 2020-04-20, Service arch 2020-05-04
-
26
The OpMsgBuilder keeps a buffer, and lets callers modify the returned BSONObj by returning an unowned BSONObjBuilder through beginBody() and resumeBody().
It looks like no size checks are ever done on the BSON before the buffer is released to a Message, which ultimately gets returned to the client. Because the BSONObjBuilders returned by beginBody() and resumeBody() are not owned, they don't perform size checks on destruction.
Here's a unit test demonstrating what I mean:
(base revision 6a764b107f77714d501b8d92f720a7853989a525)
diff --git a/src/mongo/rpc/reply_builder_test.cpp b/src/mongo/rpc/reply_builder_test.cpp
index fcdb036c65..5f89f3903e 100644
--- a/src/mongo/rpc/reply_builder_test.cpp
+++ b/src/mongo/rpc/reply_builder_test.cpp
@@ -59,6 +59,39 @@ TEST(OpMsgReplyBuilder, RoundTrip) {
testRoundTrip<rpc::OpMsgReply>(r, true);
}
+TEST(OpMsgReplyBuilder, BigRoundTrip) {
+ rpc::OpMsgReplyBuilder r;
+
+ std::string bigStr;
+ for (auto i = 0; i < 1024 * 1024 * 16; i++) {
+ bigStr += "_";
+ }
+
+ {
+ // returns an unowned BSONObjBuilder, which will not do size checking when it goes out of
+ // scope.
+ auto builder = r.getBodyBuilder();
+ for (auto i = 0; i < 2; i++) {
+ builder.append("field" + std::to_string(i), bigStr);
+ }
+ }
+
+ std::cout << "appended a lot of stuff to the builder" << std::endl;
+
+ Message replyMsg = r.done();
+ invariant(replyMsg.operation() == dbMsg);
+
+ std::cout << "serialized response to Message" << std::endl;
+
+ std::cout << "attempting to reparse Message as OpMsg" << std::endl;
+
+ // Attempt to re-parse it.
+ rpc::OpMsgReply roundTrippedResponse(&replyMsg);
+
+ // We wont' get this far.
+ MONGO_UNREACHABLE;
+}
+
template <typename T>
void testErrors(rpc::ReplyBuilderInterface& replyBuilder);
ninja +rpc_test
The test creates an OpMsgReplyBuilder, adds some absurdly large strings to the response BSON, and then attempts to round-trip it. When re-parsing the BSON a BSONObjectTooLarge error gets triggered.
It's also possible to produce this issue "in the wild." For simplicity's sake, in the below patch I have mongod return a hard-coded giant error object, but in general, error messages may contain user data.
mongos will also sometimes take all of the error objects from each shard, and cram them into one object here. This would be a more "realistic" way to encounter the issue in the wild (this is how I ran into the problem in the first place).
diff --git a/geo-repro-standalone.js b/geo-repro-standalone.js new file mode 100644 index 0000000000..a5d9273771 --- /dev/null +++ b/geo-repro-standalone.js @@ -0,0 +1,9 @@ +(function() { +const coll = db.getSiblingDB("test").coll; + +assert.commandWorked(coll.insert({a: {f: 1}})); +assert.commandWorked(coll.insert({a: {f: 1}})); + +const resp = coll.ensureIndex({a: '2d', b: 1}); +print("<ib>: response is " + tojson(resp)); +})(); diff --git a/src/mongo/db/commands/create_indexes.cpp b/src/mongo/db/commands/create_indexes.cpp index e7505ea41f..3a83ce801d 100644 --- a/src/mongo/db/commands/create_indexes.cpp +++ b/src/mongo/db/commands/create_indexes.cpp @@ -813,6 +813,24 @@ public: // If we encounter an IndexBuildAlreadyInProgress error for any of the requested index // specs, then we will wait for the build(s) to finish before trying again. const NamespaceString nss(CommandHelpers::parseNsCollectionRequired(dbname, cmdObj)); + + + if (nss.db() == "test" && nss.coll() == "coll") { + result.append("code", ErrorCodes::InternalError); + + // Return a gigantic error message. + std::string bigStr; + for (auto i = 0; i < 1024 * 1024 * 16; i++) { + bigStr += "_"; + } + + for (auto i = 0; i < 2; i++) { + result.append("field" + std::to_string(i), bigStr); + } + + return false; + } + bool shouldLogMessageOnAlreadyBuildingError = true; while (true) { try { diff --git a/src/mongo/rpc/op_msg.cpp b/src/mongo/rpc/op_msg.cpp index e0a8d60881..325cdf9155 100644 --- a/src/mongo/rpc/op_msg.cpp +++ b/src/mongo/rpc/op_msg.cpp @@ -160,6 +160,7 @@ OpMsg OpMsg::parse(const Message& message) try { case Section::kBody: { uassert(40430, "Multiple body sections in message", !haveBody); haveBody = true; + log() << "<ib>: Attempting to parse response BSON"; msg.body = sectionsBuf.read<Validated<BSONObj>>(); break; }
resmoke --suites=core geo-repro-standalone.js
The shell will hit this massert() while processing the createIndexes command response.