Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Step:FrontRunner
squid.34081
SQUID-2024_2.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File SQUID-2024_2.patch of Package squid.34081
commit 72a3bbd5e431597c3fdb56d752bc56b010ba3817 Author: Alex Rousskov <rousskov@measurement-factory.com> Date: Wed Oct 25 11:47:19 2023 +0000 Improve handling of expanding HTTP header values (#1536) Squid manipulations often increase HTTP header value length compared to the corresponding raw value received by Squid. Raw header length is checked against request_header_max_size and reply_header_max_size that default to 64KB, making the raw value safe to store in a String object (by default). However, when the increased length of a manipulated value exceeds String class limits, Squid leaks memory, asserts, or possibly stalls affected transactions. The long-term fix for this problem is a complete String elimination from Squid sources, but that takes time. Known manipulations may effectively concatenate headers and/or increase header value length by 50%. This workaround makes such known increases safe by essentially tripling String class limits: (64KB + 64KB) * 150% = 3 * 64KB This bug was discovered and detailed by Joshua Rogers at https://megamansec.github.io/Squid-Security-Audit/response-memleaks.html where it was filed as "Memory Leak in HTTP Response Parsing". diff --git a/src/SquidString.h b/src/SquidString.h index 21fdde598..ffb215fa5 100644 --- a/src/SquidString.h +++ b/src/SquidString.h @@ -140,7 +140,16 @@ private: size_type len_ = 0; /* current length */ - static const size_type SizeMax_ = 65535; ///< 64K limit protects some fixed-size buffers + /// An earlier 64KB limit was meant to protect some fixed-size buffers, but + /// (a) we do not know where those buffers are (or whether they still exist) + /// (b) too many String users unknowingly exceeded that limit and asserted. + /// We are now using a larger limit to reduce the number of (b) cases, + /// especially cases where "compact" lists of items grow 50% in size when we + /// convert them to canonical form. The new limit is selected to withstand + /// concatenation and ~50% expansion of two HTTP headers limited by default + /// request_header_max_size and reply_header_max_size settings. + static const size_type SizeMax_ = 3*64*1024 - 1; + /// 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; } diff --git a/src/cache_cf.cc b/src/cache_cf.cc index 6a0d253b1..e4a296005 100644 --- a/src/cache_cf.cc +++ b/src/cache_cf.cc @@ -1007,6 +1007,18 @@ configDoConfigure(void) (uint32_t)Config.maxRequestBufferSize, (uint32_t)Config.maxRequestHeaderSize); } + // Warn about the dangers of exceeding String limits when manipulating HTTP + // headers. Technically, we do not concatenate _requests_, so we could relax + // their check, but we keep the two checks the same for simplicity sake. + const auto safeRawHeaderValueSizeMax = (String::SizeMaxXXX()+1)/3; + // TODO: static_assert(safeRawHeaderValueSizeMax >= 64*1024); // no WARNINGs for default settings + if (Config.maxRequestHeaderSize > safeRawHeaderValueSizeMax) + debugs(3, DBG_CRITICAL, "WARNING: Increasing request_header_max_size beyond " << safeRawHeaderValueSizeMax << + " bytes makes Squid more vulnerable to denial-of-service attacks; configured value: " << Config.maxRequestHeaderSize << " bytes"); + if (Config.maxReplyHeaderSize > safeRawHeaderValueSizeMax) + debugs(3, DBG_CRITICAL, "WARNING: Increasing reply_header_max_size beyond " << safeRawHeaderValueSizeMax << + " bytes makes Squid more vulnerable to denial-of-service attacks; configured value: " << Config.maxReplyHeaderSize << " bytes"); + /* * Disable client side request pipelining if client_persistent_connections OFF. * Waste of resources queueing any pipelined requests when the first will close the connection. diff --git a/src/cf.data.pre b/src/cf.data.pre index 15e09c423..986e31499 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -6753,11 +6753,14 @@ TYPE: b_size_t DEFAULT: 64 KB LOC: Config.maxRequestHeaderSize DOC_START - This specifies the maximum size for HTTP headers in a request. - Request headers are usually relatively small (about 512 bytes). - Placing a limit on the request header size will catch certain - bugs (for example with persistent connections) and possibly - buffer-overflow or denial-of-service attacks. + This directives limits the header size of a received HTTP request + (including request-line). Increasing this limit beyond its 64 KB default + exposes certain old Squid code to various denial-of-service attacks. This + limit also applies to received FTP commands. + + This limit has no direct affect on Squid memory consumption. + + Squid does not check this limit when sending requests. DOC_END NAME: reply_header_max_size @@ -6766,11 +6769,14 @@ TYPE: b_size_t DEFAULT: 64 KB LOC: Config.maxReplyHeaderSize DOC_START - This specifies the maximum size for HTTP headers in a reply. - Reply headers are usually relatively small (about 512 bytes). - Placing a limit on the reply header size will catch certain - bugs (for example with persistent connections) and possibly - buffer-overflow or denial-of-service attacks. + This directives limits the header size of a received HTTP response + (including status-line). Increasing this limit beyond its 64 KB default + exposes certain old Squid code to various denial-of-service attacks. This + limit also applies to FTP command responses. + + Squid also checks this limit when loading hit responses from disk cache. + + Squid does not check this limit when sending responses. DOC_END NAME: request_body_max_size diff --git a/src/http.cc b/src/http.cc index f4e96aacd..138c845c7 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1900,8 +1900,9 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, String strFwd = hdr_in->getList(Http::HdrType::X_FORWARDED_FOR); - // if we cannot double strFwd size, then it grew past 50% of the limit - if (!strFwd.canGrowBy(strFwd.size())) { + // Detect unreasonably long header values. And paranoidly check String + // limits: a String ought to accommodate two reasonable-length values. + if (strFwd.size() > 32*1024 || !strFwd.canGrowBy(strFwd.size())) { // There is probably a forwarding loop with Via detection disabled. // If we do nothing, String will assert on overflow soon. // TODO: Terminate all transactions with huge XFF?
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
