From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Mon, 15 Mar 2021 14:05:05 +0000 (+0000)
Subject: Fix HttpHeaderStats definition to include hoErrorDetail (#787)
X-Git-Tag: SQUID_4_15~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f866dee2d5d944917cc339bd5fc6675d42357149;p=thirdparty%2Fsquid.git

Fix HttpHeaderStats definition to include hoErrorDetail (#787)

... when Squid is built --with-openssl.

We were "lucky" that the memory area after HttpHeaderStats was not,
apparently, used for anything important enough when HttpHeader::parse(),
indirectly called from errorInitialize() during initial Squid
configuration, was writing to it.

Detected by using AddressSanitizer.

The bug was created in commit 02259ff and cemented by commit 2673511.
---

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 4a7edc802b..e783fe13c1 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -251,6 +251,7 @@ Thank you!
     Jorge Ivan Burgos Aguilar <jorgeivanburgosaguilar@gmail.com>
     Jose Luis Godoy <joseluis.godoy@correo.aeat.es>
     Jose-Marcio Martins da Cruz <Jose-Marcio.Martins@mines-paristech.fr>
+    Joshua Rogers <megamansec@gmail.com>
     Joshua Root <jmr@macports.org>
     Joshua Root <josh+squid@root.id.au>
     JPP <jpp1@frws.com>
diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc
index 46a7d34719..8dcc7e3475 100644
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -33,6 +33,7 @@
 #include "util.h"
 
 #include <algorithm>
+#include <array>
 
 /* XXX: the whole set of API managing the entries vector should be rethought
  *      after the parse4r-ng effort is complete.
@@ -72,7 +73,7 @@ static HttpHeaderMask ReplyHeadersMask;     /* set run-time using ReplyHeaders *
 
 /* header accounting */
 // NP: keep in sync with enum http_hdr_owner_type
-static HttpHeaderStat HttpHeaderStats[] = {
+static std::array<HttpHeaderStat, hoEnd> HttpHeaderStats = {
     HttpHeaderStat(/*hoNone*/ "all", NULL),
 #if USE_HTCP
     HttpHeaderStat(/*hoHtcpReply*/ "HTCP reply", &ReplyHeadersMask),
@@ -80,11 +81,10 @@ static HttpHeaderStat HttpHeaderStats[] = {
     HttpHeaderStat(/*hoRequest*/ "request", &RequestHeadersMask),
     HttpHeaderStat(/*hoReply*/ "reply", &ReplyHeadersMask)
 #if USE_OPENSSL
-    /* hoErrorDetail */
+    , HttpHeaderStat(/*hoErrorDetail*/ "error detail templates", nullptr)
 #endif
     /* hoEnd */
 };
-static int HttpHeaderStatCount = countof(HttpHeaderStats);
 
 static int HeaderEntryParsedCount = 0;
 
@@ -127,8 +127,8 @@ httpHeaderInitModule(void)
             CBIT_SET(ReplyHeadersMask,h);
     }
 
-    /* header stats initialized by class constructor */
-    assert(HttpHeaderStatCount == hoReply + 1);
+    assert(HttpHeaderStats[0].label && "httpHeaderInitModule() called via main()");
+    assert(HttpHeaderStats[hoEnd-1].label && "HttpHeaderStats created with all elements");
 
     /* init dependent modules */
     httpHdrCcInitModule();
@@ -1607,6 +1607,9 @@ httpHeaderStatDump(const HttpHeaderStat * hs, StoreEntry * e)
     assert(hs);
     assert(e);
 
+    if (!hs->owner_mask)
+        return; // these HttpHeaderStat objects were not meant to be dumped here
+
     dump_stat = hs;
     storeAppendPrintf(e, "\nHeader Stats: %s\n", hs->label);
     storeAppendPrintf(e, "\nField type distribution\n");
@@ -1632,7 +1635,6 @@ httpHeaderStatDump(const HttpHeaderStat * hs, StoreEntry * e)
 void
 httpHeaderStoreReport(StoreEntry * e)
 {
-    int i;
     assert(e);
 
     HttpHeaderStats[0].parsedCount =
@@ -1644,9 +1646,8 @@ httpHeaderStoreReport(StoreEntry * e)
     HttpHeaderStats[0].busyDestroyedCount =
         HttpHeaderStats[hoRequest].busyDestroyedCount + HttpHeaderStats[hoReply].busyDestroyedCount;
 
-    for (i = 1; i < HttpHeaderStatCount; ++i) {
-        httpHeaderStatDump(HttpHeaderStats + i, e);
-    }
+    for (const auto &stats: HttpHeaderStats)
+        httpHeaderStatDump(&stats, e);
 
     /* field stats for all messages */
     storeAppendPrintf(e, "\nHttp Fields Stats (replies and requests)\n");