1 Fix for CVE-2016-3948. See:

     2 

     3   http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2016-3948

     4 

     5 for more details. Based on the squid version 3.5.X patch at:

     6 

     7   http://www.squid-cache.org/Versions/v3/3.5/changesets/squid-3.5-14016.patch

     8  

     9 --- squid-3.5.5/src/HttpRequest.cc.orig	2016-04-12 11:32:45.105453713 -0700

    10 +++ squid-3.5.5/src/HttpRequest.cc	2016-04-12 11:34:54.820733440 -0700

    11 @@ -92,7 +92,7 @@

    12      peer_login = NULL;      // not allocated/deallocated by this class

    13      peer_domain = NULL;     // not allocated/deallocated by this class

    14      peer_host = NULL;

    15 -    vary_headers = NULL;

    16 +    vary_headers = SBuf();

    17      myportname = null_string;

    18      tag = null_string;

    19  #if USE_AUTH

    20 @@ -124,9 +124,7 @@

    21      auth_user_request = NULL;

    22  #endif

    23      safe_free(canonical);

    24 -

    25 -    safe_free(vary_headers);

    26 -

    27 +    vary_headers.clear();

    28      url.clear();

    29      urlpath.clean();

    30  

    31 @@ -203,7 +201,7 @@

    32  

    33      copy->lastmod = lastmod;

    34      copy->etag = etag;

    35 -    copy->vary_headers = vary_headers ? xstrdup(vary_headers) : NULL;

    36 +    copy->vary_headers = vary_headers;

    37      // XXX: what to do with copy->peer_domain?

    38  

    39      copy->tag = tag;

    40 --- squid-3.5.5/src/HttpRequest.h.orig	2016-04-12 11:32:45.107463536 -0700

    41 +++ squid-3.5.5/src/HttpRequest.h	2016-04-12 11:35:03.221096498 -0700

    42 @@ -179,7 +179,8 @@

    43  

    44      time_t lastmod;     /* Used on refreshes */

    45  

    46 -    const char *vary_headers;   /* Used when varying entities are detected. Changes how the store key is calculated */

    47 +    /// The variant second-stage cache key. Generated from Vary header pattern for this request.

    48 +    SBuf vary_headers;

    49  

    50      char *peer_domain;      /* Configured peer forceddomain */

    51  

    52 --- squid-3.5.5/src/MemObject.cc.orig	2016-04-12 11:32:45.109364872 -0700

    53 +++ squid-3.5.5/src/MemObject.cc	2016-04-12 11:35:07.861297412 -0700

    54 @@ -136,8 +136,6 @@

    55      HTTPMSGUNLOCK(request);

    56  

    57      ctx_exit(ctx);              /* must exit before we free mem->url */

    58 -

    59 -    safe_free(vary_headers);

    60  }

    61  

    62  void

    63 @@ -221,8 +219,8 @@

    64  MemObject::stat(MemBuf * mb) const

    65  {

    66      mb->Printf("\t" SQUIDSBUFPH " %s\n", SQUIDSBUFPRINT(method.image()), logUri());

    67 -    if (vary_headers)

    68 -        mb->Printf("\tvary_headers: %s\n", vary_headers);

    69 +    if (!vary_headers.isEmpty())

    70 +        mb->Printf("\tvary_headers: " SQUIDSBUFPH "\n", SQUIDSBUFPRINT(vary_headers));

    71      mb->Printf("\tinmem_lo: %" PRId64 "\n", inmem_lo);

    72      mb->Printf("\tinmem_hi: %" PRId64 "\n", data_hdr.endOffset());

    73      mb->Printf("\tswapout: %" PRId64 " bytes queued\n",

    74 --- squid-3.5.5/src/MemObject.h.orig	2016-04-12 11:32:45.111075168 -0700

    75 +++ squid-3.5.5/src/MemObject.h	2016-04-12 11:35:10.909852018 -0700

    76 @@ -13,6 +13,7 @@

    77  #include "dlink.h"

    78  #include "HttpRequestMethod.h"

    79  #include "RemovalPolicy.h"

    80 +#include "SBuf.h"

    81  #include "stmem.h"

    82  #include "StoreIOBuffer.h"

    83  #include "StoreIOState.h"

    84 @@ -167,7 +168,7 @@

    85      unsigned int chksum;

    86  #endif

    87  

    88 -    const char *vary_headers;

    89 +    SBuf vary_headers;

    90  

    91      void delayRead(DeferredRead const &);

    92      void kickReads();

    93 --- squid-3.5.5/src/MemStore.cc.orig	2016-04-12 11:32:45.112589100 -0700

    94 +++ squid-3.5.5/src/MemStore.cc	2016-04-12 11:35:13.878529885 -0700

    95 @@ -443,7 +443,7 @@

    96  

    97      assert(e.mem_obj);

    98  

    99 -    if (e.mem_obj->vary_headers) {

   100 +    if (!e.mem_obj->vary_headers.isEmpty()) {

   101          // XXX: We must store/load SerialisedMetaData to cache Vary in RAM

   102          debugs(20, 5, "Vary not yet supported: " << e.mem_obj->vary_headers);

   103          return false;

   104 --- squid-3.5.5/src/StoreMetaVary.cc.orig	2016-04-12 11:32:45.114054766 -0700

   105 +++ squid-3.5.5/src/StoreMetaVary.cc	2016-04-12 11:35:17.350257452 -0700

   106 @@ -18,14 +18,14 @@

   107  {

   108      assert (getType() == STORE_META_VARY_HEADERS);

   109  

   110 -    if (!e->mem_obj->vary_headers) {

   111 +    if (e->mem_obj->vary_headers.isEmpty()) {

   112          /* XXX separate this mutator from the query */

   113          /* Assume the object is OK.. remember the vary request headers */

   114 -        e->mem_obj->vary_headers = xstrdup((char *)value);

   115 +        e->mem_obj->vary_headers.assign(static_cast<const char *>(value), length);

   116          return true;

   117      }

   118  

   119 -    if (strcmp(e->mem_obj->vary_headers, (char *)value) != 0)

   120 +    if (e->mem_obj->vary_headers.cmp(static_cast<const char *>(value), length) != 0)

   121          return false;

   122  

   123      return true;

   124 --- squid-3.5.5/src/client_side.cc.orig	2016-04-12 11:30:22.544117692 -0700

   125 +++ squid-3.5.5/src/client_side.cc	2016-04-12 11:35:20.775116943 -0700

   126 @@ -4627,20 +4627,19 @@

   127  int

   128  varyEvaluateMatch(StoreEntry * entry, HttpRequest * request)

   129  {

   130 -    const char *vary = request->vary_headers;

   131 +    SBuf vary(request->vary_headers);

   132      int has_vary = entry->getReply()->header.has(HDR_VARY);

   133  #if X_ACCELERATOR_VARY

   134 -

   135      has_vary |=

   136          entry->getReply()->header.has(HDR_X_ACCELERATOR_VARY);

   137  #endif

   138  

   139 -    if (!has_vary || !entry->mem_obj->vary_headers) {

   140 -        if (vary) {

   141 +    if (!has_vary || entry->mem_obj->vary_headers.isEmpty()) {

   142 +        if (!vary.isEmpty()) {

   143              /* Oops... something odd is going on here.. */

   144              debugs(33, DBG_IMPORTANT, "varyEvaluateMatch: Oops. Not a Vary object on second attempt, '" <<

   145                     entry->mem_obj->urlXXX() << "' '" << vary << "'");

   146 -            safe_free(request->vary_headers);

   147 +            request->vary_headers.clear();

   148              return VARY_CANCEL;

   149          }

   150  

   151 @@ -4654,8 +4653,8 @@

   152           */

   153          vary = httpMakeVaryMark(request, entry->getReply());

   154  

   155 -        if (vary) {

   156 -            request->vary_headers = xstrdup(vary);

   157 +        if (!vary.isEmpty()) {

   158 +            request->vary_headers = vary;

   159              return VARY_OTHER;

   160          } else {

   161              /* Ouch.. we cannot handle this kind of variance */

   162 @@ -4663,18 +4662,18 @@

   163              return VARY_CANCEL;

   164          }

   165      } else {

   166 -        if (!vary) {

   167 +        if (vary.isEmpty()) {

   168              vary = httpMakeVaryMark(request, entry->getReply());

   169  

   170 -            if (vary)

   171 -                request->vary_headers = xstrdup(vary);

   172 +            if (!vary.isEmpty())

   173 +                request->vary_headers = vary;

   174          }

   175  

   176 -        if (!vary) {

   177 +        if (vary.isEmpty()) {

   178              /* Ouch.. we cannot handle this kind of variance */

   179              /* XXX This cannot really happen, but just to be complete */

   180              return VARY_CANCEL;

   181 -        } else if (strcmp(vary, entry->mem_obj->vary_headers) == 0) {

   182 +        } else if (vary.cmp(entry->mem_obj->vary_headers) == 0) {

   183              return VARY_MATCH;

   184          } else {

   185              /* Oops.. we have already been here and still haven't

   186 --- squid-3.5.5/src/client_side_reply.cc.orig	2016-04-12 11:30:24.187260295 -0700

   187 +++ squid-3.5.5/src/client_side_reply.cc	2016-04-12 11:35:25.478369424 -0700

   188 @@ -988,9 +988,8 @@

   189      }

   190  

   191      /* And for Vary, release the base URI if none of the headers was included in the request */

   192 -

   193 -    if (http->request->vary_headers

   194 -            && !strstr(http->request->vary_headers, "=")) {

   195 +    if (!http->request->vary_headers.isEmpty()

   196 +            && http->request->vary_headers.find('=') != SBuf::npos) {

   197          StoreEntry *entry = storeGetPublic(urlCanonical(http->request), Http::METHOD_GET);

   198  

   199          if (entry) {

   200 --- squid-3.5.5/src/http.cc.orig	2016-04-12 11:30:25.907567935 -0700

   201 +++ squid-3.5.5/src/http.cc	2016-04-12 11:35:29.150761600 -0700

   202 @@ -573,9 +573,9 @@

   203  /*

   204   * For Vary, store the relevant request headers as

   205   * virtual headers in the reply

   206 - * Returns false if the variance cannot be stored

   207 + * Returns an empty SBuf if the variance cannot be stored

   208   */

   209 -const char *

   210 +SBuf

   211  httpMakeVaryMark(HttpRequest * request, HttpReply const * reply)

   212  {

   213      String vary, hdr;

   214 @@ -583,9 +583,9 @@

   215      const char *item;

   216      const char *value;

   217      int ilen;

   218 -    static String vstr;

   219 +    SBuf vstr;

   220 +    static const SBuf asterisk("*");

   221  

   222 -    vstr.clean();

   223      vary = reply->header.getList(HDR_VARY);

   224  

   225      while (strListGetItem(&vary, ',', &item, &ilen, &pos)) {

   226 @@ -596,11 +596,13 @@

   227          if (strcmp(name, "*") == 0) {

   228              /* Can not handle "Vary: *" withtout ETag support */

   229              safe_free(name);

   230 -            vstr.clean();

   231 +            vstr.clear();

   232              break;

   233          }

   234  

   235 -        strListAdd(&vstr, name, ',');

   236 +        if (!vstr.isEmpty())

   237 +            vstr.append(", ", 2);

   238 +        vstr.append(name);

   239          hdr = request->header.getByName(name);

   240          safe_free(name);

   241          value = hdr.termedBuf();

   242 @@ -625,7 +627,17 @@

   243          char *name = (char *)xmalloc(ilen + 1);

   244          xstrncpy(name, item, ilen + 1);

   245          Tolower(name);

   246 -        strListAdd(&vstr, name, ',');

   247 +

   248 +        if (strcmp(name, "*") == 0) {

   249 +            /* Can not handle "Vary: *" withtout ETag support */

   250 +            safe_free(name);

   251 +            vstr.clear();

   252 +            break;

   253 +        }

   254 +

   255 +        if (!vstr.isEmpty())

   256 +            vstr.append(", ", 2);

   257 +        vstr.append(name);

   258          hdr = request->header.getByName(name);

   259          safe_free(name);

   260          value = hdr.termedBuf();

   261 @@ -643,8 +655,8 @@

   262      vary.clean();

   263  #endif

   264  

   265 -    debugs(11, 3, "httpMakeVaryMark: " << vstr);

   266 -    return vstr.termedBuf();

   267 +    debugs(11, 3, vstr);

   268 +    return vstr;

   269  }

   270  

   271  void

   272 @@ -917,15 +929,15 @@

   273              || rep->header.has(HDR_X_ACCELERATOR_VARY)

   274  #endif

   275         ) {

   276 -        const char *vary = httpMakeVaryMark(request, rep);

   277 +        const SBuf vary(httpMakeVaryMark(request, rep));

   278  

   279 -        if (!vary) {

   280 +        if (vary.isEmpty()) {

   281              entry->makePrivate();

   282              if (!fwd->reforwardableStatus(rep->sline.status()))

   283                  EBIT_CLR(entry->flags, ENTRY_FWD_HDR_WAIT);

   284              varyFailure = true;

   285          } else {

   286 -            entry->mem_obj->vary_headers = xstrdup(vary);

   287 +            entry->mem_obj->vary_headers = vary;

   288          }

   289      }

   290  

   291 --- squid-3.5.5/src/http.h.orig	2016-04-12 11:30:26.674812741 -0700

   292 +++ squid-3.5.5/src/http.h	2016-04-12 11:35:34.119393224 -0700

   293 @@ -12,6 +12,7 @@

   294  #include "clients/Client.h"

   295  #include "comm.h"

   296  #include "HttpStateFlags.h"

   297 +#include "SBuf.h"

   298  

   299  class ChunkedCodingParser;

   300  class FwdState;

   301 @@ -116,7 +117,7 @@

   302  

   303  int httpCachable(const HttpRequestMethod&);

   304  void httpStart(FwdState *);

   305 -const char *httpMakeVaryMark(HttpRequest * request, HttpReply const * reply);

   306 +SBuf httpMakeVaryMark(HttpRequest * request, HttpReply const * reply);

   307  

   308  #endif /* SQUID_HTTP_H */

   309  

   310 --- squid-3.5.5/src/store.cc.orig	2016-04-12 11:30:28.336891365 -0700

   311 +++ squid-3.5.5/src/store.cc	2016-04-12 11:35:38.983063089 -0700

   312 @@ -685,31 +685,27 @@

   313      if (mem_obj->request) {

   314          HttpRequest *request = mem_obj->request;

   315  

   316 -        if (!mem_obj->vary_headers) {

   317 +        if (mem_obj->vary_headers.isEmpty()) {

   318              /* First handle the case where the object no longer varies */

   319 -            safe_free(request->vary_headers);

   320 +            request->vary_headers.clear();

   321          } else {

   322 -            if (request->vary_headers && strcmp(request->vary_headers, mem_obj->vary_headers) != 0) {

   323 +            if (!request->vary_headers.isEmpty() && request->vary_headers.cmp(mem_obj->vary_headers) != 0) {

   324                  /* Oops.. the variance has changed. Kill the base object

   325                   * to record the new variance key

   326                   */

   327 -                safe_free(request->vary_headers);       /* free old "bad" variance key */

   328 +                request->vary_headers.clear();       /* free old "bad" variance key */

   329                  if (StoreEntry *pe = storeGetPublic(mem_obj->storeId(), mem_obj->method))

   330                      pe->release();

   331              }

   332  

   333              /* Make sure the request knows the variance status */

   334 -            if (!request->vary_headers) {

   335 -                const char *vary = httpMakeVaryMark(request, mem_obj->getReply());

   336 -

   337 -                if (vary)

   338 -                    request->vary_headers = xstrdup(vary);

   339 -            }

   340 +            if (request->vary_headers.isEmpty())

   341 +                request->vary_headers = httpMakeVaryMark(request, mem_obj->getReply());

   342          }

   343  

   344          // TODO: storeGetPublic() calls below may create unlocked entries.

   345          // We should add/use storeHas() API or lock/unlock those entries.

   346 -        if (mem_obj->vary_headers && !storeGetPublic(mem_obj->storeId(), mem_obj->method)) {

   347 +        if (!mem_obj->vary_headers.isEmpty() && !storeGetPublic(mem_obj->storeId(), mem_obj->method)) {

   348              /* Create "vary" base object */

   349              String vary;

   350              StoreEntry *pe = storeCreateEntry(mem_obj->storeId(), mem_obj->logUri(), request->flags, request->method);

   351 --- squid-3.5.5/src/store_key_md5.cc.orig	2016-04-12 11:32:45.123264698 -0700

   352 +++ squid-3.5.5/src/store_key_md5.cc	2016-04-12 11:35:42.319388524 -0700

   353 @@ -125,8 +125,8 @@

   354      SquidMD5Update(&M, &m, sizeof(m));

   355      SquidMD5Update(&M, (unsigned char *) url, strlen(url));

   356  

   357 -    if (request->vary_headers) {

   358 -        SquidMD5Update(&M, (unsigned char *) request->vary_headers, strlen(request->vary_headers));

   359 +    if (!request->vary_headers.isEmpty()) {

   360 +        SquidMD5Update(&M, request->vary_headers.rawContent(), request->vary_headers.length());

   361          debugs(20, 3, "updating public key by vary headers: " << request->vary_headers << " for: " << url);

   362      }

   363  

   364 --- squid-3.5.5/src/store_swapmeta.cc.orig	2016-04-12 11:32:45.124659332 -0700

   365 +++ squid-3.5.5/src/store_swapmeta.cc	2016-04-12 11:35:45.927230596 -0700

   366 @@ -40,7 +40,6 @@

   367      tlv *TLV = NULL;        /* we'll return this */

   368      tlv **T = &TLV;

   369      const char *url;

   370 -    const char *vary;

   371      assert(e->mem_obj != NULL);

   372      const int64_t objsize = e->mem_obj->expectedReplySize();

   373      assert(e->swap_status == SWAPOUT_WRITING);

   374 @@ -87,10 +86,12 @@

   375      }

   376  

   377      T = StoreMeta::Add(T, t);

   378 -    vary = e->mem_obj->vary_headers;

   379 +    SBuf vary(e->mem_obj->vary_headers);

   380  

   381 -    if (vary) {

   382 -        t =StoreMeta::Factory(STORE_META_VARY_HEADERS, strlen(vary) + 1, vary);

   383 +    if (!vary.isEmpty()) {

   384 +        // TODO: do we still need +1 here? StoreMetaVary::checkConsistency

   385 +        //       no longer relies on nul-termination, but other things might.

   386 +        t = StoreMeta::Factory(STORE_META_VARY_HEADERS, vary.length() + 1, vary.c_str());

   387  

   388          if (!t) {

   389              storeSwapTLVFree(TLV);

   390 --- squid-3.5.5/src/tests/stub_MemObject.cc.orig	2016-04-12 11:34:21.726285840 -0700

   391 +++ squid-3.5.5/src/tests/stub_MemObject.cc	2016-04-12 11:35:49.383455312 -0700

   392 @@ -38,7 +38,6 @@

   393      id(0),

   394      object_sz(-1),

   395      swap_hdr_sz(0),

   396 -    vary_headers(NULL),

   397      _reply(NULL)

   398  {

   399      memset(&clients, 0, sizeof(clients));

   400 --- squid-3.5.5/src/tests/stub_http.cc.orig	2016-04-12 11:34:21.728074407 -0700

   401 +++ squid-3.5.5/src/tests/stub_http.cc	2016-04-12 11:35:52.784226579 -0700

   402 @@ -7,12 +7,12 @@

   403   */

   404  

   405  #include "squid.h"

   406 -

   407  #include "HttpReply.h"

   408  #include "HttpRequest.h"

   409 +#include "SBuf.h"

   410  

   411  #define STUB_API "http.cc"

   412  #include "tests/STUB.h"

   413  

   414 -const char * httpMakeVaryMark(HttpRequest * request, HttpReply const * reply) STUB_RETVAL(NULL)

   415 +SBuf httpMakeVaryMark(HttpRequest *, HttpReply const *) STUB_RETVAL(SBuf())

   416