From 337cf19d1084fd4cef4d0a4bf27797eef3b266dc Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Thu, 23 Jul 2026 14:00:41 -0400 Subject: [PATCH 1/7] Handle even more partial responses This workaround continues the work started in 2021 commits 8af775ed and 7024fb73. A comprehensive long-term fix requires backlogged Client Streams removal followed by noisy StoreIoBuffer and HttpHdrContRange API changes. --- src/HttpHdrContRange.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/HttpHdrContRange.cc b/src/HttpHdrContRange.cc index ebfd03fcaba..a81e756a470 100644 --- a/src/HttpHdrContRange.cc +++ b/src/HttpHdrContRange.cc @@ -15,6 +15,7 @@ #include "HttpHdrContRange.h" #include "HttpHeaderTools.h" +#include /* * Currently only byte ranges are supported * @@ -184,6 +185,19 @@ httpHdrContRangeParseInit(HttpHdrContRange * range, const char *str) return 0; } + // We increase offset by the size of various objects/buffers (e.g., Store + // metadata, serialized HTTP headers, mem_node::data, and Store I/O buffer). + // Those increases do not check for overflows yet, so limit the offset here + // while assuming that none of those sizes would ever exceed maximumSize. + // We further assume that most offsets use int64_t or a larger type. + static_assert(std::numeric_limitsspec.offset)>::max() >= std::numeric_limits::max()); + const auto maximumSize = int64_t(1024)*1024*1024*1024; // no in-memory Squid object/buffer size can exceed 1 TiB + const auto maximumOffset = std::numeric_limits::max() - maximumSize; + if (range->spec.length > maximumOffset || range->spec.offset > maximumOffset - range->spec.length) { + debugs(68, 2, "huge content-range-spec near: '" << str << "'"); + return 0; + } + debugs(68, 8, "parsed content-range field: " << (long int) range->spec.offset << "-" << (long int) range->spec.offset + range->spec.length - 1 << " / " << From 28b78e9d67923ce2f2410ec49ba28b4406d9572d Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Thu, 23 Jul 2026 14:23:54 -0400 Subject: [PATCH 2/7] fixup: Polished --- src/HttpHdrContRange.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/HttpHdrContRange.cc b/src/HttpHdrContRange.cc index a81e756a470..412a96005e3 100644 --- a/src/HttpHdrContRange.cc +++ b/src/HttpHdrContRange.cc @@ -16,6 +16,7 @@ #include "HttpHeaderTools.h" #include + /* * Currently only byte ranges are supported * @@ -185,11 +186,11 @@ httpHdrContRangeParseInit(HttpHdrContRange * range, const char *str) return 0; } - // We increase offset by the size of various objects/buffers (e.g., Store - // metadata, serialized HTTP headers, mem_node::data, and Store I/O buffer). - // Those increases do not check for overflows yet, so limit the offset here - // while assuming that none of those sizes would ever exceed maximumSize. - // We further assume that most offsets use int64_t or a larger type. + // Store I/O adds partial content offsets to the size of various objects and + // buffers (e.g., Store metadata, serialized HTTP headers, mem_node::data, + // and Store I/O buffer). Most such sums do not check for overflows, so we + // check here while assuming that those sizes cannot exceed maximumSize. We + // further assume that most offsets use int64_t or a larger integer type. static_assert(std::numeric_limitsspec.offset)>::max() >= std::numeric_limits::max()); const auto maximumSize = int64_t(1024)*1024*1024*1024; // no in-memory Squid object/buffer size can exceed 1 TiB const auto maximumOffset = std::numeric_limits::max() - maximumSize; From 73506d0934114810ada38eba040bc29a39baa131 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Thu, 23 Jul 2026 14:23:10 -0400 Subject: [PATCH 3/7] fixup: Almost missed a nearby sum that needs our protection! --- src/HttpHdrContRange.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/HttpHdrContRange.cc b/src/HttpHdrContRange.cc index 412a96005e3..c651bc7479a 100644 --- a/src/HttpHdrContRange.cc +++ b/src/HttpHdrContRange.cc @@ -175,9 +175,6 @@ httpHdrContRangeParseInit(HttpHdrContRange * range, const char *str) /* Additional paranoidal check for BUG2155 - entity-length MUST be > 0 */ debugs(68, 2, "invalid (entity-length is negative) content-range-spec near: '" << str << "'"); return 0; - } else if (known_spec(range->spec.length) && range->elength < (range->spec.offset + range->spec.length)) { - debugs(68, 2, "invalid (range is outside entity-length) content-range-spec near: '" << str << "'"); - return 0; } // reject unsatisfied-range and such; we only use well-defined ranges today @@ -199,6 +196,11 @@ httpHdrContRangeParseInit(HttpHdrContRange * range, const char *str) return 0; } + if (range->elength < (range->spec.offset + range->spec.length)) { + debugs(68, 2, "invalid (range is outside entity-length) content-range-spec near: '" << str << "'"); + return 0; + } + debugs(68, 8, "parsed content-range field: " << (long int) range->spec.offset << "-" << (long int) range->spec.offset + range->spec.length - 1 << " / " << From 619829e45c258c0be633e38440e109030ce840b3 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Thu, 23 Jul 2026 15:21:05 -0400 Subject: [PATCH 4/7] fixup: Almost missed an even earlier sum that needs our protection! * resp-range-spec case was protected by subsequent size_diff()<=0 check. * Request range-spec case resulted in incorrect/negative length despite successful HttpHdrRangeSpec::parseInit() outcome. I do not know what the consequences of that inconsistency are. --- src/HttpHdrContRange.cc | 5 +++++ src/HttpHdrRange.cc | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/HttpHdrContRange.cc b/src/HttpHdrContRange.cc index c651bc7479a..ca4b32aa38f 100644 --- a/src/HttpHdrContRange.cc +++ b/src/HttpHdrContRange.cc @@ -88,6 +88,11 @@ httpHdrRangeRespSpecParseInit(HttpHdrRangeSpec * spec, const char *field, int fl return 0; } + if (last_pos == std::numeric_limits::max()) { + debugs(68, 2, "unsupported huge last-byte-pos resp-range-spec near: '" << field << "'"); + return 0; + } + spec->length = size_diff(last_pos + 1, spec->offset); /* we managed to parse, check if the result makes sense */ diff --git a/src/HttpHdrRange.cc b/src/HttpHdrRange.cc index c8be30f6287..e80f8001ac2 100644 --- a/src/HttpHdrRange.cc +++ b/src/HttpHdrRange.cc @@ -96,6 +96,11 @@ HttpHdrRangeSpec::parseInit(const char *field, int flen) return false; } + if (last_pos == std::numeric_limits::max()) { + debugs(64, 2, "unsupported huge last-byte-pos range-spec near: " << field); + return false; + } + HttpHdrRangeSpec::HttpRange aSpec (offset, last_pos + 1); length = aSpec.size(); From 8a293c4adc8e967eebcd725658eea9b9ac374017 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Thu, 23 Jul 2026 16:16:43 -0400 Subject: [PATCH 5/7] fixup: Belt and suspenders The cast is necessary to avoid compiler `-Wsign-compare` warnings because `wrote` is unsigned. TODO: Consider adding and using Increment[able](offset, delta) and/or reusing existing IncreaseSum(offset, delta). --- src/stmem.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/stmem.cc b/src/stmem.cc index a7ea833e9ed..eae7c6f9e8c 100644 --- a/src/stmem.cc +++ b/src/stmem.cc @@ -321,7 +321,9 @@ mem_hdr::write (StoreIOBuffer const &writeBuffer) while (len && (target = nodeToRecieve(currentOffset))) { size_t wrote = writeAvailable(target, currentOffset, len, currentSource); assert (wrote); + Assure(len >= wrote); len -= wrote; + Assure(currentOffset <= std::numeric_limits::max() - static_cast(wrote)); currentOffset += wrote; currentSource += wrote; } From e67d8840cd9eae361d5c563e0a9218baff6753d4 Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Thu, 23 Jul 2026 16:36:08 -0400 Subject: [PATCH 6/7] fixup: A better version of the same check This version is not just more compact: It is checking inputs _before_ `mem_hdr::write()` modifies anything. Its primary problem is that it cannot distinguish overflowing `writeBuffer.offset + writeBuffer.length` sum from negative `writeBuffer.length`. However, it can be argued that all three of those are likely the result of integer overflowing in caller's code. N.B. `unionNotEmpty()` call asserts that offset is not negative, so this change does not really drop that `mem_hdr::write()` parameter check. --- src/stmem.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stmem.cc b/src/stmem.cc index eae7c6f9e8c..5923665ed9e 100644 --- a/src/stmem.cc +++ b/src/stmem.cc @@ -13,6 +13,7 @@ #include "HttpReply.h" #include "mem_node.h" #include "MemObject.h" +#include "SquidMath.h" #include "stmem.h" /* @@ -311,7 +312,7 @@ mem_hdr::write (StoreIOBuffer const &writeBuffer) return false; } - assert (writeBuffer.offset >= 0); + Assure(IncreaseSum(writeBuffer.offset, writeBuffer.length)); mem_node *target; int64_t currentOffset = writeBuffer.offset; @@ -323,7 +324,6 @@ mem_hdr::write (StoreIOBuffer const &writeBuffer) assert (wrote); Assure(len >= wrote); len -= wrote; - Assure(currentOffset <= std::numeric_limits::max() - static_cast(wrote)); currentOffset += wrote; currentSource += wrote; } From 82a28b15135427bc5e0008451e8879896f13944b Mon Sep 17 00:00:00 2001 From: Alex Rousskov Date: Fri, 24 Jul 2026 14:37:13 -0400 Subject: [PATCH 7/7] fixup: Do not compare with unknown elength I was tricked by the complex structure of the chained `if` statements from where the problematic test had to be moved. The `*` case does set `range->elength` to `range_spec_unknown` while other cases reject negative `elength` values. --- src/HttpHdrContRange.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HttpHdrContRange.cc b/src/HttpHdrContRange.cc index ca4b32aa38f..536fee9ff22 100644 --- a/src/HttpHdrContRange.cc +++ b/src/HttpHdrContRange.cc @@ -201,7 +201,7 @@ httpHdrContRangeParseInit(HttpHdrContRange * range, const char *str) return 0; } - if (range->elength < (range->spec.offset + range->spec.length)) { + if (known_spec(range->elength) && range->elength < (range->spec.offset + range->spec.length)) { debugs(68, 2, "invalid (range is outside entity-length) content-range-spec near: '" << str << "'"); return 0; }