From: Tomek Mrugalski <tomasz@isc.org>
Date: Mon, 13 May 2019 11:45:56 +0000 (+0200)
Subject: [#599] Mitigation patch
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fheads%2F599-assert-mitigation;p=thirdparty%2Fkea.git

[#599] Mitigation patch

 We don't understand the exact nature of the problem yet, but this
 change should prevent the server from asserting and continue its
 operation.
---

diff --git a/src/lib/http/client.cc b/src/lib/http/client.cc
index b1474fb4e7..8ff0e879d8 100644
--- a/src/lib/http/client.cc
+++ b/src/lib/http/client.cc
@@ -535,10 +535,18 @@ Connection::terminate(const boost::system::error_code& ec,
                   HTTP_SERVER_RESPONSE_RECEIVED)
             .arg(url_.toText());
 
+        // It's unlikely, but possible that we got here due to a timeout and that
+        // we actually received data that was not processed yet. We don't understand
+        // the exact nature of the problem yet, but this is one suspected place
+        // where we process the timeout callback first (that sets parser_ to null),
+        // and then we attempt to process receiveCallback that attempts to
+        // call terminate(), but the parser_ is already null. So this code
+        // below used to assert.
         LOG_DEBUG(http_logger, isc::log::DBGLVL_TRACE_BASIC_DATA,
                   HTTP_SERVER_RESPONSE_RECEIVED_DETAILS)
             .arg(url_.toText())
-            .arg(parser_->getBufferAsString(MAX_LOGGED_MESSAGE_SIZE));
+            .arg(parser_ ? parser_->getBufferAsString(MAX_LOGGED_MESSAGE_SIZE)
+                 : "[HttpResponseParser is null]");
 
     } else {
         std::string err = parsing_error.empty() ? ec.message() : parsing_error;
@@ -550,7 +558,7 @@ Connection::terminate(const boost::system::error_code& ec,
 
         // Only log the details if we have received anything and tried
         // to parse it.
-        if (!parsing_error.empty()) {
+        if (parser_ && !parsing_error.empty()) {
             LOG_DEBUG(http_logger, isc::log::DBGLVL_TRACE_BASIC_DATA,
                       HTTP_BAD_SERVER_RESPONSE_RECEIVED_DETAILS)
                 .arg(url_.toText())
@@ -704,6 +712,14 @@ Connection::receiveCallback(const boost::system::error_code& ec, size_t length)
     // Receiving is in progress, so push back the timeout.
     scheduleTimer(timer_.getInterval());
 
+    // This should never happen. However, if we got here and somehow the parser_
+    // is null, we need to terminate. If this happens, it's a programming error.
+    // Nevertheless, this allows us to recover and continue server operation.
+    if (!parser_) {
+        terminate(boost::asio::error::fault, "Logic error: HttpResponseParserPtr is null");
+        return;
+    }
+
     // If we have received any data, let's feed the parser with it.
     if (length != 0) {
         parser_->postBuffer(static_cast<void*>(input_buf_.data()), length);