path: root/main/squid/squid-3.5-13991-CVE-2016-2569.patch

patch was modified for applying to squid-3.4.14
------------------------------------------------------------
revno: 13991
revision-id: rousskov@measurement-factory.com-20160219231541-syrgnvl1av8bbn8d
parent: rousskov@measurement-factory.com-20160218041533-8tmtd45c3nky2gyy
committer: Alex Rousskov <rousskov@measurement-factory.com>
branch nick: 3.5
timestamp: Fri 2016-02-19 16:15:41 -0700
message:
  Throw instead of asserting on some String overflows.
  
  Note that Client-caught exceptions result in HTTP 500 (Internal Server
  Error) responses with X-Squid-Error set to "ERR_CANNOT_FORWARD 0".
  
  Also avoid stuck Client jobs on exceptions. 
  
  Also unified String size limit checks.
  
  Essentially trunk r14552, which has a detailed commit message.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: rousskov@measurement-factory.com-20160219231541-\
#   syrgnvl1av8bbn8d
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: 3a9c41e0584065e737250cf9f8eb9eea7a85e9ba
# timestamp: 2016-02-19 23:50:57 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: rousskov@measurement-factory.com-20160218041533-\
#   8tmtd45c3nky2gyy
# 
# Begin patch
=== modified file 'src/SquidString.h'
--- a/src/SquidString.h
+++ b/src/SquidString.h
@@ -80,6 +80,13 @@
     _SQUID_INLINE_ int caseCmp(char const *, size_type count) const;
     _SQUID_INLINE_ int caseCmp(String const &) const;
 
+    /// Whether creating a totalLen-character string is safe (i.e., unlikely to assert).
+    /// Optional extras can be used for overflow-safe length addition.
+    /// Implementation has to add 1 because many String allocation methods do.
+    static bool CanGrowTo(size_type totalLen, const size_type extras = 0) { return SafeAdd(totalLen, extras) && SafeAdd(totalLen, 1); }
+    /// whether appending growthLen characters is safe (i.e., unlikely to assert)
+    bool canGrowBy(const size_type growthLen) const { return CanGrowTo(size(), growthLen); }
+
     String substr(size_type from, size_type to) const;
 
     _SQUID_INLINE_ void cut(size_type newLength);
@@ -95,10 +102,14 @@
     _SQUID_INLINE_ bool nilCmp(bool, bool, int &) const;
 
     /* never reference these directly! */
-    size_type size_; /* buffer size; 64K limit */
+    size_type size_; /* buffer size; limited by SizeMax_ */
 
     size_type len_;  /* current length  */
 
+    static const size_type SizeMax_ = 65535; ///< 64K limit protects some fixed-size buffers
+    /// returns true after increasing the first argument by extra if the sum does not exceed SizeMax_
+    static bool SafeAdd(size_type &base, size_type extra) { if (extra <= SizeMax_ && base <= SizeMax_ - extra) { base += extra; return true; } return false; }
+
     char *buf_;
 
     _SQUID_INLINE_ void set(char const *loc, char const ch);

=== modified file 'src/StrList.cc'
--- a/src/StrList.cc
+++ b/src/StrList.cc
@@ -11,20 +11,24 @@
 #include "squid.h"
 #include "SquidString.h"
 #include "StrList.h"
+#include "base/TextException.h" 
 
 /** appends an item to the list */
 void
 strListAdd(String * str, const char *item, char del)
 {
     assert(str && item);
+    const String::size_type itemSize = strlen(item);
     if (str->size()) {
         char buf[3];
         buf[0] = del;
         buf[1] = ' ';
         buf[2] = '\0';
+        Must(str->canGrowBy(2));
         str->append(buf, 2);
     }
-    str->append(item, strlen(item));
+    Must(str->canGrowBy(itemSize));
+    str->append(item, itemSize);
 }
 
 /** returns true iff "m" is a member of the list */

=== modified file 'src/String.cc'
--- a/src/String.cc
+++ b/src/String.cc
@@ -42,7 +42,7 @@
 String::setBuffer(char *aBuf, String::size_type aSize)
 {
     assert(undefined());
-    assert(aSize < 65536);
+    assert(aSize <= SizeMax_);
     buf_ = aBuf;
     size_ = aSize;
 }
@@ -171,7 +171,7 @@
     } else {
         // Create a temporary string and absorb it later.
         String snew;
-        assert(len_ + len < 65536); // otherwise snew.len_ overflows below
+        assert(canGrowBy(len)); // otherwise snew.len_ may overflow below
         snew.len_ = len_ + len;
         snew.allocBuffer(snew.len_ + 1);
 

=== modified file 'src/Server.cc'
--- a/src/Server.cc
+++ b/src/Server.cc
@@ -49,6 +49,7 @@
         startedAdaptation(false),
 #endif
         receivedWholeRequestBody(false),
+        doneWithFwd(NULL),
         theVirginReply(NULL),
         theFinalReply(NULL)
 {
@@ -74,8 +75,6 @@
     HTTPMSGUNLOCK(theVirginReply);
     HTTPMSGUNLOCK(theFinalReply);
 
-    fwd = NULL; // refcounted
-
     if (responseBodyBuffer != NULL) {
         delete responseBodyBuffer;
         responseBodyBuffer = NULL;
@@ -93,6 +92,14 @@
     cleanAdaptation();
 #endif
 
+    if (!doneWithServer())
+        closeServer();
+
+    if (!doneWithFwd) {
+        doneWithFwd = "swanSong()";
+        fwd->handleUnregisteredServerEnd();
+    }
+
     BodyConsumer::swanSong();
 #if USE_ADAPTATION
     Initiator::swanSong();
@@ -218,6 +225,7 @@
 {
     debugs(11,5, HERE << "completing forwarding for "  << fwd);
     assert(fwd != NULL);
+    doneWithFwd = "completeForwarding()";
     fwd->complete();
 }
 

=== modified file 'src/Server.h'
--- a/src/Server.h
+++ b/src/Server.h
@@ -176,6 +176,10 @@
 #endif
     bool receivedWholeRequestBody; ///< handleRequestBodyProductionEnded called
 
+    /// whether we should not be talking to FwdState; XXX: clear fwd instead
+    /// points to a string literal which is used only for debugging
+    const char *doneWithFwd;
+
 private:
     void sendBodyIsTooLargeError();
     void maybePurgeOthers();

=== modified file 'src/ftp.cc'
--- a/src/ftp.cc
+++ b/src/ftp.cc
@@ -839,6 +839,7 @@
 {
     debugs(9, 4, HERE);
     ctrl.clear();
+    doneWithFwd = "ctrlClosed()"; // assume FwdState is monitoring too
     mustStop("FtpStateData::ctrlClosed");
 }
 

=== modified file 'src/http.cc'
--- a/src/http.cc
+++ b/src/http.cc
@@ -152,6 +152,7 @@
 HttpStateData::httpStateConnClosed(const CommCloseCbParams &params)
 {
     debugs(11, 5, "httpStateFree: FD " << params.fd << ", httpState=" << params.data);
+    doneWithFwd = "httpStateConnClosed()"; // assume FwdState is monitoring too
     mustStop("HttpStateData::httpStateConnClosed");
 }
 
@@ -2407,21 +2409,11 @@
     ServerStateData::sentRequestBody(io);
 }
 
-// Quickly abort the transaction
-// TODO: destruction should be sufficient as the destructor should cleanup,
-// including canceling close handlers
 void
 HttpStateData::abortTransaction(const char *reason)
 {
     debugs(11,5, HERE << "aborting transaction for " << reason <<
            "; " << serverConnection << ", this " << this);
-
-    if (Comm::IsConnOpen(serverConnection)) {
-        serverConnection->close();
-        return;
-    }
-
-    fwd->handleUnregisteredServerEnd();
-    mustStop("HttpStateData::abortTransaction");
+    mustStop(reason);
 }