Fix Certificate Validator buffer-overflow crashes Squid (commit r14757)

- In the case of unexpected or timedout helper replies we must ignore the
  current message untill an eom found.

- Inside helperStatefulHandleRead, the current Helper::Xaction can be NULL
  in the case of unexpected helper reply.

This is a Measurement Factory Project

diff --git a/src/helper.cc b/src/helper.cc
index 9991b2362aa1fd600d1022e565c3a2a900b67a25..fe52569d5b9c70d87f08800808f1ea30c2eeca87 100644 (file)
--- a/src/helper.cc
+++ b/src/helper.cc
@@ -208,6 +208,7 @@ helperOpenServers(helper * hlp)
         srv->roffset = 0;
         srv->nextRequestId = 0;
         srv->replyXaction = NULL;
+        srv->ignoreToEom = false;
         srv->parent = cbdataReference(hlp);
         dlinkAddTail(srv, &srv->link, &hlp->servers);
 
@@ -945,7 +946,7 @@ helperHandleRead(const Comm::ConnectionPointer &conn, char *, size_t len, Comm::
             *eom = '\0';
         }
 
-        if (!srv->replyXaction) {
+        if (!srv->ignoreToEom && !srv->replyXaction) {
             int i = 0;
             if (hlp->childs.concurrency) {
                 char *e = NULL;
@@ -967,6 +968,7 @@ helperHandleRead(const Comm::ConnectionPointer &conn, char *, size_t len, Comm::
                            i << " from " << hlp->id_name << " #" << srv->index <<
                            " '" << srv->rbuf << "'");
                 }
+                srv->ignoreToEom = true;
             }
         } // else we need to just append reply data to the current Xaction
 
@@ -976,6 +978,10 @@ helperHandleRead(const Comm::ConnectionPointer &conn, char *, size_t len, Comm::
             helperReturnBuffer(srv, hlp, msg, msgSize, eom);
             msg += msgSize + skip;
             assert(static_cast<size_t>(msg - srv->rbuf) <= srv->rbuf_sz);
+
+            // The next message should not ignored.
+            if (eom && srv->ignoreToEom)
+                srv->ignoreToEom = false;
         } else
             assert(skip == 0 && eom == NULL);
     }
@@ -1052,7 +1058,7 @@ helperStatefulHandleRead(const Comm::ConnectionPointer &conn, char *, size_t len
         *t = '\0';
     }
 
-    if (!r->reply.accumulate(srv->rbuf, t ? (t - srv->rbuf) : srv->roffset)) {
+    if (r && !r->reply.accumulate(srv->rbuf, t ? (t - srv->rbuf) : srv->roffset)) {
         debugs(84, DBG_IMPORTANT, "ERROR: Disconnecting from a " <<
                "helper that overflowed " << srv->rbuf_sz << "-byte " <<
                "Squid input buffer: " << hlp->id_name << " #" << srv->index);

diff --git a/src/helper.h b/src/helper.h
index 2e74c00e05f019a08bad202d738cc7cf5c83780b..04e79cde9bf9ac0bab5cc9b1e67ee239f1051037 100644 (file)
--- a/src/helper.h
+++ b/src/helper.h
@@ -221,6 +221,9 @@ public:
     /// the end-of-message for current reply is not retrieved.
     Helper::Xaction *replyXaction;
 
+    /// Whether to ignore current message, because it is timed-out or other reason
+    bool ignoreToEom;
+
     // STL says storing std::list iterators is safe when changing the list
     typedef std::map<uint64_t, Requests::iterator> RequestIndex;
     RequestIndex requestsIndex; ///< maps request IDs to requests