Bug Fix: Squid may crash, when accessing an SSL certificate with errors

This is a security bug.
The bug report is:
   When accessing a revoked certificate (i.e., X509_V_ERR_CERT_REVOKED) Squid
crashes. In ssl/ErrorDetail.cc:223 the detailed message is left blank if the
error is not specifically handled by Squid and the errorpage.cc:1193 assertion
fails while trying to convert the message.

This patch:
 - Handle inside ErrorState::Convert the cases where the ErrorDetail return
   blank error detail string
 - Use a default  detail error message in ssl::ErrorDetail, for the cases
   where the detail error is not defined in TheSslDetailMap.
 - If a name for the ssl::ErrorDetail error code is not defined return the
   numeric error code when the "%err_name" formating code used

diff --git a/src/errorpage.cc b/src/errorpage.cc
index c2479a4d9738957bf540344bfab3da8f9ba728b1..280b16219feb2c9715cc8f5cd6cee697d10f3a15 100644 (file)
--- a/src/errorpage.cc
+++ b/src/errorpage.cc
@@ -687,12 +687,15 @@ ErrorState::Convert(char token, bool building_deny_info_url, bool allowRecursion
         // currently only SSL error details implemented
         else if (detail) {
             const String &errDetail = detail->toString();
-            MemBuf *detail_mb  = ConvertText(errDetail.termedBuf(), false);
-            mb.append(detail_mb->content(), detail_mb->contentSize());
-            delete detail_mb;
-            do_quote = 0;
-        } else
+            if (errDetail.defined()) {
+                MemBuf *detail_mb  = ConvertText(errDetail.termedBuf(), false);
+                mb.append(detail_mb->content(), detail_mb->contentSize());
+                delete detail_mb;
+                do_quote = 0;
+            }
+        }
 #endif
+        if (!mb.contentSize())
             mb.Printf("[No Error Detail]");
         break;
 

diff --git a/src/ssl/ErrorDetail.cc b/src/ssl/ErrorDetail.cc
index a939ad1291215c33620b8f211c0fd03728f412c1..6bc194bf7821e8149885785dec73a9015189e214 100644 (file)
--- a/src/ssl/ErrorDetail.cc
+++ b/src/ssl/ErrorDetail.cc
@@ -7,6 +7,7 @@ struct SslErrorDetailEntry {
     const char *detail;
 };
 
+static const char *SslErrorDetailDefaultStr = "SSL certificate validation error (%err_name): %ssl_subject";
 // TODO: optimize by replacing with std::map or similar
 static SslErrorDetailEntry TheSslDetailMap[] = {
     {  SQUID_X509_V_ERR_DOMAIN_MISMATCH,
@@ -77,7 +78,9 @@ static const char *getErrorDetail(Ssl::ssl_error_t value)
             return TheSslDetailMap[i].detail;
     }
 
-    return NULL;
+    // we must always return something because ErrorDetail::buildDetail
+    // will hit an assertion
+    return SslErrorDetailDefaultStr;
 }
 
 Ssl::ErrorDetail::err_frm_code Ssl::ErrorDetail::ErrorFormatingCodes[] = {
@@ -176,9 +179,12 @@ const char *Ssl::ErrorDetail::notafter() const
  */
 const char *Ssl::ErrorDetail::err_code() const
 {
+    static char tmpBuffer[64];
     const char *err = getErrorName(error_no);
-    if (!err)
-        return "[Not available]";
+    if (!err) {
+        snprintf(tmpBuffer, 64, "%d", (int)error_no); 
+        err = tmpBuffer;
+    }
     return err;
 }
 
@@ -220,9 +226,7 @@ void Ssl::ErrorDetail::buildDetail() const
     char const *t;
     int code_len = 0;
 
-    if (!s)  //May be add a default detail string?
-        return;
-
+    assert(s);
     while ((p = strchr(s, '%'))) {
         errDetailStr.append(s, p - s);
         code_len = convert(++p, &t);