[SERVER-13517] internal client should validate BSON responses Created: 08/Apr/14  Updated: 02/Sep/16  Resolved: 19/Aug/16

Status: Closed
Project: Core Server
Component/s: Internal Client, Networking
Affects Version/s: 2.2.7, 2.6.0
Fix Version/s: 3.3.12

Type: Bug Priority: Major - P3
Reporter: Benety Goh Assignee: Adam Chelminski (Inactive)
Resolution: Done Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Platforms 2016-08-26
Participants:
Linked BF Score: 0

 Description   

This was found on a 2.2 server but the defect still exists in master. The mongod was running a replica set heartbeat command against another node but received some corrupted BSON in the results. On closer inspection, it appears validateBSON is not called in this code path.

Here's a demangled stack trace:

... [rsHealthPoll] Assertion: 10320:BSONElement: bad type -51
0xaffd31 0xac5eb9 0x57105b 0x5a631d 0x592ac6 0x5a86e7 0x5a8b51 0x94f5c5 0x9555c2 0x955dd8 0xacd6ce 0xac8dfe 0xaca444 0xb45ba9 0x7f3ae7921e9a 0x7f3ae6c36dbd
/usr/bin/mongod(mongo::printStackTrace(std::basic_ostream<char, std::char_traits<char> >&)+0x21) [0xaffd31]
/usr/bin/mongod(mongo::msgasserted(int, char const*)+0x99) [0xac5eb9]
/usr/bin/mongod(mongo::BSONElement::size() const+0x1cb) [0x57105b]
/usr/bin/mongod(mongo::BSONObj::getField(mongo::StringData const&) const+0x7d) [0x5a631d]
/usr/bin/mongod(mongo::DBClientWithCommands::isOk(mongo::BSONObj const&)+0x26) [0x592ac6]
/usr/bin/mongod(mongo::DBClientWithCommands::runCommand(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, mongo::BSONObj const&, mongo::BSONObj&, int, mongo::AuthenticationTable const*)+0x2f7) [0x5a86e7]
/usr/bin/mongod(mongo::DBClientConnection::runCommand(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, mongo::BSONObj const&, mongo::BSONObj&, int, mongo::AuthenticationTable const*)+0x11) [0x5a8b51]
/usr/bin/mongod(mongo::requestHeartbeat(std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, mongo::BSONObj&, int, int&, bool)+0x565) [0x94f5c5]
/usr/bin/mongod(mongo::ReplSetHealthPollTask::_requestHeartbeat(mongo::HeartbeatInfo&, mongo::BSONObj&, int&)+0xf2) [0x9555c2]
/usr/bin/mongod(mongo::ReplSetHealthPollTask::doWork()+0xa8) [0x955dd8]
/usr/bin/mongod(mongo::task::Task::run()+0x1e) [0xacd6ce]
/usr/bin/mongod(mongo::BackgroundJob::jobBody(boost::shared_ptr<mongo::BackgroundJob::JobStatus>)+0xbe) [0xac8dfe]
/usr/bin/mongod(boost::detail::thread_data<boost::_bi::bind_t<void, boost::_mfi::mf1<void, mongo::BackgroundJob, boost::shared_ptr<mongo::BackgroundJob::JobStatus> >, boost::_bi::list2<boost::_bi::value<mongo::BackgroundJob*>, boost::_bi::value<boost::shared_ptr<mongo::BackgroundJob::JobStatus> > > > >::run()+0x74) [0xaca444]
/usr/bin/mongod() [0xb45ba9]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x7e9a) [0x7f3ae7921e9a]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x6d) [0x7f3ae6c36dbd]



 Comments   
Comment by Githook User [ 19/Aug/16 ]

Author:

{u'username': u'adamchel', u'name': u'Adam Chelminski', u'email': u'adam.chelminski@mongodb.com'}

Message: SERVER-13517 Internal client validates BSON in all command responses and when reading from cursor in DBClientCursor
Branch: master
https://github.com/mongodb/mongo/commit/1a3d60af4d27d72e15637bb43510fe1b592a6c43

Comment by Githook User [ 15/Aug/16 ]

Author:

{u'username': u'adamchel', u'name': u'Adam Chelminski', u'email': u'adam.chelminski@mongodb.com'}

Message: Revert "SERVER-13517 Specialize operator<< for BSONType for std::ostream, LogstreamBuilder, and StringBuilder"

This reverts commit 0b2645558c9715128dceb524660b603e9d8532d6.
Branch: master
https://github.com/mongodb/mongo/commit/d19d5cdb3a2dd6905361545239c7c37d07cf5aba

Comment by Githook User [ 15/Aug/16 ]

Author:

{u'username': u'adamchel', u'name': u'Adam Chelminski', u'email': u'adam.chelminski@mongodb.com'}

Message: SERVER-13517 Specialize operator<< for BSONType for std::ostream, LogstreamBuilder, and StringBuilder
Branch: master
https://github.com/mongodb/mongo/commit/0b2645558c9715128dceb524660b603e9d8532d6

Comment by Eric Milkie [ 12/Apr/16 ]

I think William wanted my first option presented above, which is that the C++ driver validates incoming responses. As far as I know, the internal client does not do this today.

Comment by Ian Whalen (Inactive) [ 11/Apr/16 ]

milkie since the last comment here have we added the hardening/validation that William was asking about?

Comment by William Zola [ 16/Apr/14 ]

The issue here is that invalid incoming BSON from administrative commands (such as replication pings or the internal sharding commands) can cause the server to crash non-obviously. In an environment where the network silently corrupts the incoming BSON, this will cause the 'mongod' to crash.

The request is to "harden" the server such that invalid or corrupt incoming BSON – no matter what the source – does not crash the 'mongod' or 'mongos'.

Comment by Benety Goh [ 09/Apr/14 ]

The issue we observed in the logs is related to validating the response from the server.

Comment by Eric Milkie [ 08/Apr/14 ]

Are you saying the C++ driver should validate the response from the server, or should the server validate outgoing responses? I'm not sure we should have either. Right now (2.4+) the server validates incoming BSON, but that wouldn't help in this situation. If you need data checking on your network links, one option is to use TLS, which will break the connection at a lower level reliably. Validating BSON as a substitute for parity checking won't catch many errors.

Generated at Thu Feb 08 03:31:59 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.