------------------------------------------------------------
revno: 13846
revision-id: squid3@treenet.co.nz-20150605234642-9awa9h9gxnc79bap
parent: squid3@treenet.co.nz-20150605234122-cc64o3mlr8qqk1f5
author: Christos Tsantilas <chtsanti@users.sourceforge.net>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Fri 2015-06-05 16:46:42 -0700
message:
  Fix assertion String.cc:221: "str"
  
  This bug can be caused by certificates does not contain a CN field. In this
  case the Ssl::ErrorDetail::cn method may return NULL causing this assertion
  somewhere inside Ssl::ErrorDetail::buildDetail method, which expects always
  a non NULL value from Ssl::ErrorDetail::cn and similar methods.
  
  This patch try to hardening the Ssl::ErrorDetail error formating functions to
  avoid always check for NULL values and also avoid sending wrong information
  for various certificate fields in the case of an error while extracting the
  information from certificate..
  
  This is a Measurement Factory project
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20150605234642-9awa9h9gxnc79bap
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: b8d2bc0b31b716d4ff8529236381697847247da3
# timestamp: 2015-06-05 23:51:04 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20150605234122-\
#   cc64o3mlr8qqk1f5
# 
# Begin patch
=== modified file 'src/ssl/ErrorDetail.cc'
--- src/ssl/ErrorDetail.cc	2015-01-13 09:13:49 +0000
+++ src/ssl/ErrorDetail.cc	2015-06-05 23:46:42 +0000
@@ -430,13 +430,12 @@
  */
 const char  *Ssl::ErrorDetail::subject() const
 {
-    if (!broken_cert)
-        return "[Not available]";
-
-    static char tmpBuffer[256]; // A temporary buffer
-    X509_NAME_oneline(X509_get_subject_name(broken_cert.get()), tmpBuffer,
-                      sizeof(tmpBuffer));
-    return tmpBuffer;
+    if (broken_cert.get()) {
+        static char tmpBuffer[256]; // A temporary buffer
+        if (X509_NAME_oneline(X509_get_subject_name(broken_cert.get()), tmpBuffer, sizeof(tmpBuffer)))
+            return tmpBuffer;
+    }
+    return "[Not available]";
 }
 
 // helper function to be used with Ssl::matchX509CommonNames
@@ -445,9 +444,11 @@
     String *str = (String *)check_data;
     if (!str) // no data? abort
         return 0;
-    if (str->size() > 0)
-        str->append(", ");
-    str->append((const char *)cn_data->data, cn_data->length);
+    if (cn_data && cn_data->length) {
+        if (str->size() > 0)
+            str->append(", ");
+        str->append((const char *)cn_data->data, cn_data->length);
+    }
     return 1;
 }
 
@@ -456,13 +457,14 @@
  */
 const char *Ssl::ErrorDetail::cn() const
 {
-    if (!broken_cert)
-        return "[Not available]";
-
-    static String tmpStr;  ///< A temporary string buffer
-    tmpStr.clean();
-    Ssl::matchX509CommonNames(broken_cert.get(), &tmpStr, copy_cn);
-    return tmpStr.termedBuf();
+    if (broken_cert.get()) {
+        static String tmpStr;  ///< A temporary string buffer
+        tmpStr.clean();
+        Ssl::matchX509CommonNames(broken_cert.get(), &tmpStr, copy_cn);
+        if (tmpStr.size())
+            return tmpStr.termedBuf();
+    }
+    return "[Not available]";
 }
 
 /**
@@ -470,12 +472,12 @@
  */
 const char *Ssl::ErrorDetail::ca_name() const
 {
-    if (!broken_cert)
-        return "[Not available]";
-
-    static char tmpBuffer[256]; // A temporary buffer
-    X509_NAME_oneline(X509_get_issuer_name(broken_cert.get()), tmpBuffer, sizeof(tmpBuffer));
-    return tmpBuffer;
+    if (broken_cert.get()) {
+        static char tmpBuffer[256]; // A temporary buffer
+        if (X509_NAME_oneline(X509_get_issuer_name(broken_cert.get()), tmpBuffer, sizeof(tmpBuffer)))
+            return tmpBuffer;
+    }
+    return "[Not available]";
 }
 
 /**
@@ -483,13 +485,14 @@
  */
 const char *Ssl::ErrorDetail::notbefore() const
 {
-    if (!broken_cert)
-        return "[Not available]";
-
-    static char tmpBuffer[256]; // A temporary buffer
-    ASN1_UTCTIME * tm = X509_get_notBefore(broken_cert.get());
-    Ssl::asn1timeToString(tm, tmpBuffer, sizeof(tmpBuffer));
-    return tmpBuffer;
+    if (broken_cert.get()) {
+        if (ASN1_UTCTIME * tm = X509_get_notBefore(broken_cert.get())) {
+            static char tmpBuffer[256]; // A temporary buffer
+            Ssl::asn1timeToString(tm, tmpBuffer, sizeof(tmpBuffer));
+            return tmpBuffer;
+        }
+    }
+    return "[Not available]";
 }
 
 /**
@@ -497,13 +500,14 @@
  */
 const char *Ssl::ErrorDetail::notafter() const
 {
-    if (!broken_cert)
-        return "[Not available]";
-
-    static char tmpBuffer[256]; // A temporary buffer
-    ASN1_UTCTIME * tm = X509_get_notAfter(broken_cert.get());
-    Ssl::asn1timeToString(tm, tmpBuffer, sizeof(tmpBuffer));
-    return tmpBuffer;
+    if (broken_cert.get()) {
+        if (ASN1_UTCTIME * tm = X509_get_notAfter(broken_cert.get())) {
+            static char tmpBuffer[256]; // A temporary buffer
+            Ssl::asn1timeToString(tm, tmpBuffer, sizeof(tmpBuffer));
+            return tmpBuffer;
+        }
+    }
+    return "[Not available]";
 }
 
 /**