From c402339bbc301ad8ee27132c18bb31c3bf3f9ed5 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Mon, 15 Jun 2026 23:45:02 +0100 Subject: [PATCH 01/16] tests: characterize extacl base64 stack buffer overflow Adds testExtAclBase64Overflow, a CppUnit test suite that confirms the stack buffer overflow present in three production sites: src/adaptation/icap/ModXact.cc makeRequestHeaders() (site A) src/http.cc httpFixupAuthentication() PASS/PROXYPASS (site B) src/http.cc httpFixupAuthentication() '*'-prefix path (site C) All three allocate a 175-byte stack buffer (base64_encode_len(MAX_LOGIN_SZ)) then pass unchecked extacl_user/extacl_passwd sizes to base64_encode_update(). When a helper returns values totalling >= 130 bytes, the encoded output (176 bytes) overflows the buffer. The tests replicate each encoding pattern using an oversized scratch buffer to count emitted bytes without triggering undefined behaviour, then assert that the byte count exceeds the production allocation. Ten cases are covered: buffer-size anchor, safe/boundary/overflow inputs for sites A&B, the exact 130-byte overflow threshold, large-input confirmation, and site C's two-segment (user + peer_login_suffix) path. --- src/Makefile.am | 21 ++ src/tests/testExtAclBase64Overflow.cc | 316 ++++++++++++++++++++++++++ 2 files changed, 337 insertions(+) create mode 100644 src/tests/testExtAclBase64Overflow.cc diff --git a/src/Makefile.am b/src/Makefile.am index 4480764023d..966f45da0c5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1994,6 +1994,27 @@ tests_testHttpRequest_LDADD = \ $(XTRA_LIBS) tests_testHttpRequest_LDFLAGS = $(LIBADD_DL) +## Characterization test: extacl base64 stack buffer overflow (CVE candidate) +## Confirms that extacl_user/extacl_passwd values exceeding MAX_LOGIN_SZ +## cause base64 output to overflow the fixed-size stack buffers in +## ModXact.cc::makeRequestHeaders() and http.cc::httpFixupAuthentication(). + +check_PROGRAMS += tests/testExtAclBase64Overflow +tests_testExtAclBase64Overflow_SOURCES = \ + tests/testExtAclBase64Overflow.cc +nodist_tests_testExtAclBase64Overflow_SOURCES = \ + $(TESTSOURCES) \ + tests/stub_debug.cc \ + tests/stub_libmem.cc +tests_testExtAclBase64Overflow_LDADD = \ + $(top_builddir)/lib/libmiscencoding.la \ + base/libbase.la \ + $(LIBCPPUNIT_LIBS) \ + $(COMPAT_LIB) \ + $(LIBNETTLE_LIBS) \ + $(XTRA_LIBS) +tests_testExtAclBase64Overflow_LDFLAGS = $(LIBADD_DL) + ## Tests of ip/* check_PROGRAMS += tests/testIpAddress diff --git a/src/tests/testExtAclBase64Overflow.cc b/src/tests/testExtAclBase64Overflow.cc new file mode 100644 index 00000000000..c0d2d91b5e9 --- /dev/null +++ b/src/tests/testExtAclBase64Overflow.cc @@ -0,0 +1,316 @@ +/* + * Copyright (C) 1996-2026 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +/* + * Characterization tests that confirm the stack buffer overflow in the + * external-ACL credential base64 encoding paths. + * + * Three production sites allocate a fixed-size stack buffer sized for + * MAX_LOGIN_SZ bytes of pre-encoding input: + * + * src/adaptation/icap/ModXact.cc (makeRequestHeaders) — site A + * src/http.cc httpFixupAuthentication() PASS/PROXYPASS branch — site B + * src/http.cc httpFixupAuthentication() '*'-prefix branch — site C + * + * Sites A and B encode: extacl_user + ":" + extacl_passwd + * Site C encodes: extacl_user + peer_login_suffix + * + * All three pass the actual (unchecked) field sizes directly to + * base64_encode_update() without verifying that the combined plain-text + * length fits within MAX_LOGIN_SZ. When a helper returns values whose + * combined encoded length exceeds base64_encode_len(MAX_LOGIN_SZ), the + * writes overflow the fixed-size stack buffer. + * + * These tests replicate the exact encoding sequences used at each site + * and assert that the bytes written exceed the buffer allocation + * whenever the input exceeds the safe threshold. No actual + * out-of-bounds write is performed: we count required output bytes + * using a generously-sized scratch buffer and compare against the + * allocation that the production code would have used. + * + * Safe input threshold: + * Sites A/B: user + ":" + passwd must total < 130 bytes (i.e. <= 129). + * Site C: user + peer_login_suffix must total < 130 bytes. + * At 130 bytes the encoded output (176 bytes) exceeds the 175-byte buffer. + */ + +#include "squid.h" +#include "compat/cppunit.h" +#include "defines.h" +#include "unitTestMain.h" + +#include "base64.h" + +#include +#include +#include +#include + +/// The fixed-size stack buffer used in all three vulnerable sites. +/// base64_encode_len(128) = ((128*8+4)/6) + 3 + 1 = 171 + 3 + 1 = 175. +static constexpr size_t ProductionBufSize = base64_encode_len(MAX_LOGIN_SZ); + +/// Replicates the encoding pattern used in: +/// - ModXact.cc::makeRequestHeaders() +/// - httpFixupAuthentication() PASS/PROXYPASS branch (sites A and B) +/// +/// Encodes: user + ":" + passwd (three-segment pattern) +static size_t +encodeUserColon(const char *user, size_t userLen, + const char *passwd, size_t passwdLen) +{ + // Scratch buffer large enough for any input we test — avoids UB while + // faithfully counting emitted bytes. + std::vector scratch(8192, '\0'); + char *dst = scratch.data(); + + struct base64_encode_ctx ctx; + base64_encode_init(&ctx); + + size_t total = 0; + total += base64_encode_update(&ctx, dst + total, userLen, + reinterpret_cast(user)); + total += base64_encode_update(&ctx, dst + total, 1, + reinterpret_cast(":")); + total += base64_encode_update(&ctx, dst + total, passwdLen, + reinterpret_cast(passwd)); + total += base64_encode_final(&ctx, dst + total); + return total; +} + +/// Replicates the encoding pattern used in: +/// - httpFixupAuthentication() '*'-prefix branch (site C) +/// +/// Encodes: username + peerLoginSuffix (two-segment pattern, no ':') +static size_t +encodeUserSuffix(const char *username, size_t usernameLen, + const char *suffix, size_t suffixLen) +{ + std::vector scratch(8192, '\0'); + char *dst = scratch.data(); + + struct base64_encode_ctx ctx; + base64_encode_init(&ctx); + + size_t total = 0; + total += base64_encode_update(&ctx, dst + total, usernameLen, + reinterpret_cast(username)); + total += base64_encode_update(&ctx, dst + total, suffixLen, + reinterpret_cast(suffix)); + total += base64_encode_final(&ctx, dst + total); + return total; +} + +class TestExtAclBase64Overflow : public CPPUNIT_NS::TestFixture +{ + CPPUNIT_TEST_SUITE(TestExtAclBase64Overflow); + // Buffer-size anchor + CPPUNIT_TEST(testBufferSizeCalculation); + // Sites A & B — user + ":" + passwd pattern + CPPUNIT_TEST(testSafeInputFitsBuffer); + CPPUNIT_TEST(testMaxSafeBoundaryFits); + CPPUNIT_TEST(testExactOverflowThreshold); + CPPUNIT_TEST(testOversizedUserOverflows); + CPPUNIT_TEST(testOversizedPasswdOverflows); + CPPUNIT_TEST(testCombinedOverflow); + CPPUNIT_TEST(testLargeInputOverflows); + // Site C — user + peer_login_suffix pattern + CPPUNIT_TEST(testSiteC_SafeInputFitsBuffer); + CPPUNIT_TEST(testSiteC_OversizedUserOverflows); + CPPUNIT_TEST_SUITE_END(); + +protected: + /// Confirm the production buffer is exactly 175 bytes and MAX_LOGIN_SZ is 128. + void testBufferSizeCalculation(); + + // ── Sites A & B: user + ":" + passwd ───────────────────────────────── + + /// Well-within-limit input does not overflow. + void testSafeInputFitsBuffer(); + + /// user=63, passwd=64: combined plain=128 bytes — fits with 3 bytes spare. + void testMaxSafeBoundaryFits(); + + /// user=64, passwd=65: combined plain=130 bytes — the minimal overflow. + /// base64(130 bytes) = 176 bytes > 175-byte buffer. + void testExactOverflowThreshold(); + + /// user alone exceeds MAX_LOGIN_SZ — sites A and B overflow. + void testOversizedUserOverflows(); + + /// passwd alone exceeds MAX_LOGIN_SZ — sites A and B overflow. + void testOversizedPasswdOverflows(); + + /// Moderate combined excess (65+64=129+":"=130) — both sites overflow. + void testCombinedOverflow(); + + /// Very large input — confirms no arithmetic wrapping masks the overflow. + void testLargeInputOverflows(); + + // ── Site C: user + peer_login_suffix ───────────────────────────────── + + /// Small user + small suffix stays within buffer. + void testSiteC_SafeInputFitsBuffer(); + + /// Oversized extacl_user overflows the same loginbuf via site C's path. + void testSiteC_OversizedUserOverflows(); +}; + +CPPUNIT_TEST_SUITE_REGISTRATION(TestExtAclBase64Overflow); + +void +TestExtAclBase64Overflow::testBufferSizeCalculation() +{ + // base64_encode_len(128): + // BASE64_ENCODE_LENGTH(128) = (128×8 + 4) / 6 = 1028/6 = 171 (integer division) + // BASE64_ENCODE_FINAL_LENGTH = 3 + // NUL sentinel = 1 + // = 175 + CPPUNIT_ASSERT_EQUAL(static_cast(175), ProductionBufSize); + CPPUNIT_ASSERT_EQUAL(static_cast(128), static_cast(MAX_LOGIN_SZ)); +} + +void +TestExtAclBase64Overflow::testSafeInputFitsBuffer() +{ + // user="alice" (5), passwd="secret" (6): total plain = 12 bytes — well under 128. + const char *user = "alice"; + const char *passwd = "secret"; + const auto written = encodeUserColon(user, strlen(user), passwd, strlen(passwd)); + + CPPUNIT_ASSERT_MESSAGE("safe credentials must fit in the fixed stack buffer", + written <= ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testMaxSafeBoundaryFits() +{ + // user=63 + ":" + passwd=64 = 128 bytes total — exactly MAX_LOGIN_SZ. + // BASE64_ENCODE_RAW_LENGTH(128) = ((128+2)/3)*4 = 43*4 = 172 bytes written. + // 172 <= 175: fits with 3 bytes to spare. + const std::string user(63, 'U'); + const std::string passwd(64, 'P'); + const auto written = encodeUserColon(user.c_str(), user.size(), + passwd.c_str(), passwd.size()); + + CPPUNIT_ASSERT_MESSAGE("input of exactly MAX_LOGIN_SZ bytes must fit in fixed buffer", + written <= ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testExactOverflowThreshold() +{ + // user=64 + ":" + passwd=65 = 130 bytes total — the minimal overflowing input. + // BASE64_ENCODE_RAW_LENGTH(130) = ((130+2)/3)*4 = 44*4 = 176 bytes. + // 176 > 175: the first input size that actually overflows the buffer. + const std::string user(64, 'U'); + const std::string passwd(65, 'P'); + const auto written = encodeUserColon(user.c_str(), user.size(), + passwd.c_str(), passwd.size()); + + CPPUNIT_ASSERT_MESSAGE("130-byte combined input (user+colon+passwd) is the minimal overflow", + written > ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testOversizedUserOverflows() +{ + // user=129 bytes alone (+ ":" + empty passwd = 130 bytes total). + // The user field exceeds MAX_LOGIN_SZ; production code at sites A and B + // passes user.size() = 129 directly to base64_encode_update() without + // checking, writing into a 175-byte stack buffer. + // Total plain = 130 bytes => 176 bytes encoded > 175 bytes available. + const std::string user(129, 'A'); + const char *passwd = ""; + const auto written = encodeUserColon(user.c_str(), user.size(), passwd, strlen(passwd)); + + CPPUNIT_ASSERT_MESSAGE("oversized user field (site A: ModXact, site B: httpFixupAuthentication) " + "causes encoded output to exceed fixed stack buffer", + written > ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testOversizedPasswdOverflows() +{ + // user="u" (1 byte) + ":" + passwd=129 bytes = 131 bytes total. + // passwd alone exceeds MAX_LOGIN_SZ; passed unchecked at sites A and B. + // 131 bytes encoded = 176 bytes > 175 bytes. + const char *user = "u"; + const std::string passwd(129, 'P'); + const auto written = encodeUserColon(user, strlen(user), passwd.c_str(), passwd.size()); + + CPPUNIT_ASSERT_MESSAGE("oversized passwd field (site A: ModXact, site B: httpFixupAuthentication) " + "causes encoded output to exceed fixed stack buffer", + written > ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testCombinedOverflow() +{ + // user=65 + ":" + passwd=64 = 130 bytes total. + // Each field looks reasonable in isolation (both < MAX_LOGIN_SZ) but + // their combination exceeds the safe threshold by 2 bytes. + const std::string user(65, 'U'); + const std::string passwd(64, 'P'); + const auto written = encodeUserColon(user.c_str(), user.size(), + passwd.c_str(), passwd.size()); + + CPPUNIT_ASSERT_MESSAGE("combined user+colon+passwd exceeding safe threshold " + "overflows fixed stack buffer", + written > ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testLargeInputOverflows() +{ + // 512+512 = 1024 bytes; confirms no size-type wrapping masks the overflow. + const std::string user(512, 'A'); + const std::string passwd(512, 'B'); + const auto written = encodeUserColon(user.c_str(), user.size(), + passwd.c_str(), passwd.size()); + + CPPUNIT_ASSERT_MESSAGE("very large input must also overflow fixed stack buffer", + written > ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testSiteC_SafeInputFitsBuffer() +{ + // Site C (httpFixupAuthentication '*'-prefix branch) encodes: + // extacl_user + peer_login_suffix (no ':') + // into the same loginbuf[base64_encode_len(MAX_LOGIN_SZ)] = 175 bytes. + // Small inputs fit fine. + const char *username = "alice"; + const char *suffix = "@realm"; + const auto written = encodeUserSuffix(username, strlen(username), + suffix, strlen(suffix)); + + CPPUNIT_ASSERT_MESSAGE("site C: small extacl_user + peer_login_suffix must fit in fixed buffer", + written <= ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testSiteC_OversizedUserOverflows() +{ + // Site C: extacl_user = 129 bytes + any non-empty peer_login_suffix. + // Total plain = 130 bytes => 176 bytes encoded > 175-byte loginbuf. + const std::string username(129, 'U'); + const char *suffix = "x"; // minimal non-empty suffix + const auto written = encodeUserSuffix(username.c_str(), username.size(), + suffix, strlen(suffix)); + + CPPUNIT_ASSERT_MESSAGE("site C: oversized extacl_user causes encoded output to exceed " + "fixed loginbuf in httpFixupAuthentication '*'-prefix path", + written > ProductionBufSize); +} + +int +main(int argc, char *argv[]) +{ + return TestProgram().run(argc, argv); +} From 2125dc5ef4087a42caeadff9657000d9c51433a6 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Wed, 17 Jun 2026 23:17:18 +0100 Subject: [PATCH 02/16] icap: fix extacl credential buffer overflow in makeRequestHeaders When extacl_user and extacl_passwd are set by an external ACL helper, makeRequestHeaders() encodes them as a Basic Proxy-Authorization credential into a fixed 175-byte stack buffer: char base64buf[base64_encode_len(MAX_LOGIN_SZ)]; // 175 bytes The encoding calls previously passed extacl_user.size() and extacl_passwd.size() directly to base64_encode_update() without checking that the combined plain-text length fits within MAX_LOGIN_SZ (128 bytes). A helper returning values totalling >= 130 bytes causes the encoded output (>= 176 bytes) to overflow the 175-byte stack buffer. Fix: add a pre-encoding guard that throws a TextException when plainLen (user + ':' + passwd) exceeds MAX_LOGIN_SZ. The exception propagates through the existing JobDialer catch, calls ModXact::callException(), tears down the ICAP transaction cleanly, and returns an error answer to the HTTP client. The guard uses the same throw-on-violation pattern already used elsewhere in ModXact.cc. The guard is deliberately conservative (fires at plainLen > 128, i.e., >= 129) to ensure the 3-byte base64_encode_final padding slot is always available within the buffer. Update testExtAclBase64Overflow to document the guard's threshold: - testGuardAllowsExactLimit: plainLen=128 must not fire the guard - testGuardRejectsOneByteOver: plainLen=129 triggers the guard (conservative; actual encoding overflow starts at plainLen=130) --- src/adaptation/icap/ModXact.cc | 14 +++- src/tests/testExtAclBase64Overflow.cc | 106 +++++++++++++++++++++----- 2 files changed, 100 insertions(+), 20 deletions(-) diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index 5f23e52395c..890ac44c255 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -1399,12 +1399,20 @@ void Adaptation::Icap::ModXact::makeRequestHeaders(MemBuf &buf) String vh=virgin.header->header.getById(Http::HdrType::PROXY_AUTHORIZATION); buf.appendf("Proxy-Authorization: " SQUIDSTRINGPH "\r\n", SQUIDSTRINGPRINT(vh)); } else if (request->extacl_user.size() > 0 && request->extacl_passwd.size() > 0) { + const auto userLen = request->extacl_user.size(); + const auto passwdLen = request->extacl_passwd.size(); + // +1 for the ':' separator between user and passwd + const auto plainLen = userLen + 1 + passwdLen; + if (plainLen > MAX_LOGIN_SZ) + throw TexcHere("extacl credentials too long for Proxy-Authorization"); + // plainLen <= MAX_LOGIN_SZ, so base64_encode_len(plainLen) fits + // within the base64_encode_len(MAX_LOGIN_SZ) stack buffer. + char base64buf[base64_encode_len(MAX_LOGIN_SZ)]; struct base64_encode_ctx ctx; base64_encode_init(&ctx); - char base64buf[base64_encode_len(MAX_LOGIN_SZ)]; - size_t resultLen = base64_encode_update(&ctx, base64buf, request->extacl_user.size(), reinterpret_cast(request->extacl_user.rawBuf())); + size_t resultLen = base64_encode_update(&ctx, base64buf, userLen, reinterpret_cast(request->extacl_user.rawBuf())); resultLen += base64_encode_update(&ctx, base64buf+resultLen, 1, reinterpret_cast(":")); - resultLen += base64_encode_update(&ctx, base64buf+resultLen, request->extacl_passwd.size(), reinterpret_cast(request->extacl_passwd.rawBuf())); + resultLen += base64_encode_update(&ctx, base64buf+resultLen, passwdLen, reinterpret_cast(request->extacl_passwd.rawBuf())); resultLen += base64_encode_final(&ctx, base64buf+resultLen); buf.appendf("Proxy-Authorization: Basic %.*s\r\n", (int)resultLen, base64buf); } diff --git a/src/tests/testExtAclBase64Overflow.cc b/src/tests/testExtAclBase64Overflow.cc index c0d2d91b5e9..d508d1032b3 100644 --- a/src/tests/testExtAclBase64Overflow.cc +++ b/src/tests/testExtAclBase64Overflow.cc @@ -7,8 +7,8 @@ */ /* - * Characterization tests that confirm the stack buffer overflow in the - * external-ACL credential base64 encoding paths. + * Characterization and fix-verification tests for the stack buffer overflow + * in the external-ACL credential base64 encoding paths. * * Three production sites allocate a fixed-size stack buffer sized for * MAX_LOGIN_SZ bytes of pre-encoding input: @@ -20,23 +20,27 @@ * Sites A and B encode: extacl_user + ":" + extacl_passwd * Site C encodes: extacl_user + peer_login_suffix * - * All three pass the actual (unchecked) field sizes directly to - * base64_encode_update() without verifying that the combined plain-text - * length fits within MAX_LOGIN_SZ. When a helper returns values whose - * combined encoded length exceeds base64_encode_len(MAX_LOGIN_SZ), the - * writes overflow the fixed-size stack buffer. + * Without a guard, all three pass the actual (unchecked) field sizes + * directly to base64_encode_update() into a 175-byte stack buffer. + * When a helper returns values whose combined plain-text length exceeds + * 129 bytes the encoded output (>= 176 bytes) overflows the buffer. * - * These tests replicate the exact encoding sequences used at each site - * and assert that the bytes written exceed the buffer allocation - * whenever the input exceeds the safe threshold. No actual - * out-of-bounds write is performed: we count required output bytes - * using a generously-sized scratch buffer and compare against the - * allocation that the production code would have used. + * The fix for site A (ModXact.cc) adds a pre-encoding guard: * - * Safe input threshold: - * Sites A/B: user + ":" + passwd must total < 130 bytes (i.e. <= 129). - * Site C: user + peer_login_suffix must total < 130 bytes. - * At 130 bytes the encoded output (176 bytes) exceeds the 175-byte buffer. + * if (plainLen > MAX_LOGIN_SZ) // i.e. > 128 + * throw TexcHere(...); + * + * The buffer-overflow section of this file characterizes the encoding + * arithmetic (showing the overflow would occur without the guard). + * The guard-condition section verifies the fix's threshold directly. + * + * Buffer overflow threshold (encoding arithmetic): + * At 130 bytes of plain input the encoded output is 176 bytes, + * which exceeds the 175-byte buffer by 1 byte. + * + * Guard threshold (site A fix): + * plainLen > MAX_LOGIN_SZ (128) triggers the throw. + * plainLen == 128 is safe; plainLen == 129 is rejected. */ #include "squid.h" @@ -122,6 +126,9 @@ class TestExtAclBase64Overflow : public CPPUNIT_NS::TestFixture // Site C — user + peer_login_suffix pattern CPPUNIT_TEST(testSiteC_SafeInputFitsBuffer); CPPUNIT_TEST(testSiteC_OversizedUserOverflows); + // Site A fix — guard condition (ModXact.cc: plainLen > MAX_LOGIN_SZ throws) + CPPUNIT_TEST(testGuardAllowsExactLimit); + CPPUNIT_TEST(testGuardRejectsOneByteOver); CPPUNIT_TEST_SUITE_END(); protected: @@ -159,6 +166,18 @@ class TestExtAclBase64Overflow : public CPPUNIT_NS::TestFixture /// Oversized extacl_user overflows the same loginbuf via site C's path. void testSiteC_OversizedUserOverflows(); + + // ── Site A fix: guard condition verification ────────────────────────── + // The fix in ModXact.cc guards: if (plainLen > MAX_LOGIN_SZ) throw ... + // These tests verify the arithmetic of that guard directly. + + /// plainLen == MAX_LOGIN_SZ (128) must NOT trigger the guard. + /// Encodes correctly within the 175-byte buffer (172 bytes written). + void testGuardAllowsExactLimit(); + + /// plainLen == MAX_LOGIN_SZ+1 (129) MUST trigger the guard. + /// Without the guard this input would write 176 bytes into a 175-byte buffer. + void testGuardRejectsOneByteOver(); }; CPPUNIT_TEST_SUITE_REGISTRATION(TestExtAclBase64Overflow); @@ -309,6 +328,59 @@ TestExtAclBase64Overflow::testSiteC_OversizedUserOverflows() written > ProductionBufSize); } +void +TestExtAclBase64Overflow::testGuardAllowsExactLimit() +{ + // The site A guard condition is: if (plainLen > MAX_LOGIN_SZ) throw + // plainLen == MAX_LOGIN_SZ (128) must NOT trigger it. + // + // user=63 + ":" + passwd=64 = 128 bytes. + // BASE64_ENCODE_RAW_LENGTH(128) = 172 bytes written; 172 <= 175. Safe. + const std::string user(63, 'U'); + const std::string passwd(64, 'P'); + const auto plainLen = user.size() + 1 + passwd.size(); + CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ), plainLen); + + const auto written = encodeUserColon(user.c_str(), user.size(), + passwd.c_str(), passwd.size()); + CPPUNIT_ASSERT_MESSAGE("plainLen == MAX_LOGIN_SZ must not overflow the buffer " + "(guard must allow this input)", + written <= ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testGuardRejectsOneByteOver() +{ + // The site A guard condition is: if (plainLen > MAX_LOGIN_SZ) throw + // plainLen == MAX_LOGIN_SZ+1 (129) MUST trigger the guard. + // + // Note: the guard is deliberately conservative. At plainLen=129 the + // encoded output is exactly 175 bytes (== ProductionBufSize), which + // technically fits. However the guard fires at plainLen > MAX_LOGIN_SZ, + // rejecting 129 to keep a safe margin for the 3-byte final padding slot. + // The true minimal overflowing input is 130 bytes (see testExactOverflowThreshold). + // + // This test confirms: + // (a) plainLen == MAX_LOGIN_SZ + 1 (inputs constructed correctly) + // (b) the guard condition (plainLen > MAX_LOGIN_SZ) is TRUE + // (c) the encoded output still fits (guard fires before actual overflow) + const std::string user(64, 'U'); + const std::string passwd(64, 'P'); + const auto plainLen = user.size() + 1 + passwd.size(); + CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ + 1), plainLen); + + // (b) guard fires: plainLen > MAX_LOGIN_SZ + CPPUNIT_ASSERT_MESSAGE("plainLen == MAX_LOGIN_SZ+1 must trigger the guard condition", + plainLen > static_cast(MAX_LOGIN_SZ)); + + // (c) at this size the encoder writes exactly ProductionBufSize bytes (no margin) + const auto written = encodeUserColon(user.c_str(), user.size(), + passwd.c_str(), passwd.size()); + CPPUNIT_ASSERT_MESSAGE("plainLen == MAX_LOGIN_SZ+1 still fits in the buffer " + "(guard is conservative; actual overflow starts at plainLen=130)", + written <= ProductionBufSize); +} + int main(int argc, char *argv[]) { From 3d1055f90d92d17a68a64ec42bef97abf43b4a5f Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Wed, 17 Jun 2026 23:28:41 +0100 Subject: [PATCH 03/16] http: fix extacl credential buffer overflows in httpFixupAuthentication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit httpFixupAuthentication() shares a single 175-byte stack buffer (loginbuf[base64_encode_len(MAX_LOGIN_SZ)]) across three encoding branches, none of which previously checked that the plain-text input fit within MAX_LOGIN_SZ (128 bytes) before writing. Affected branches and their plain-text inputs: Site B — PASS/PROXYPASS: extacl_user + ':' + extacl_passwd Both fields are helper-controlled; identical overflow to site A. Guard: if (userLen + 1 + passwdLen > MAX_LOGIN_SZ) throw Site C — '*'-prefix: extacl_user + peer_login suffix after '*' Username is helper-controlled; suffix is operator-controlled. Guard: if (usernameLen + suffixLen > MAX_LOGIN_SZ) throw Fallback — plain peer_login literal Entirely operator-controlled, but uses the same buffer. Guard: if (loginLen > MAX_LOGIN_SZ) throw All three guards use throw TexcHere(...), which propagates through JobDialer::dial()'s catch to AsyncJob::callException() -> mustStop("exception"), tearing the HTTP-to-peer connection down cleanly and returning an error to the client. This is the same pattern used in the site A fix (ModXact.cc) and throughout http.cc. The guards are deliberately conservative (fire at > MAX_LOGIN_SZ, i.e. >= 129) matching the site A fix, ensuring the 3-byte base64_encode_final padding slot is always available. Extend testExtAclBase64Overflow with four new cases (16 total): testSiteB_GuardAllowsExactLimit — plainLen=128 does not fire testSiteB_GuardRejectsOneByteOver — plainLen=129 fires (conservative) testSiteC_GuardAllowsExactLimit — username+suffix=128 does not fire testSiteC_GuardRejectsOneByteOver — username+suffix=129 fires --- src/http.cc | 22 ++++-- src/tests/testExtAclBase64Overflow.cc | 100 ++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/src/http.cc b/src/http.cc index 82c90586bbf..de941d451b3 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1850,8 +1850,12 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe username = request->auth_user_request->username(); #endif - blen = base64_encode_update(&ctx, loginbuf, strlen(username), reinterpret_cast(username)); - blen += base64_encode_update(&ctx, loginbuf+blen, strlen(request->peer_login +1), reinterpret_cast(request->peer_login +1)); + const auto usernameLen = strlen(username); + const auto suffixLen = strlen(request->peer_login + 1); + if (usernameLen + suffixLen > MAX_LOGIN_SZ) + throw TexcHere("peer login credentials too long"); + blen = base64_encode_update(&ctx, loginbuf, usernameLen, reinterpret_cast(username)); + blen += base64_encode_update(&ctx, loginbuf+blen, suffixLen, reinterpret_cast(request->peer_login +1)); blen += base64_encode_final(&ctx, loginbuf+blen); httpHeaderPutStrf(hdr_out, header, "Basic %.*s", (int)blen, loginbuf); return; @@ -1862,9 +1866,14 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe (strcmp(request->peer_login, "PASS") == 0 || strcmp(request->peer_login, "PROXYPASS") == 0)) { - blen = base64_encode_update(&ctx, loginbuf, request->extacl_user.size(), reinterpret_cast(request->extacl_user.rawBuf())); + const auto userLen = request->extacl_user.size(); + const auto passwdLen = request->extacl_passwd.size(); + // +1 for the ':' separator between user and passwd + if (userLen + 1 + passwdLen > MAX_LOGIN_SZ) + throw TexcHere("extacl credentials too long for peer login"); + blen = base64_encode_update(&ctx, loginbuf, userLen, reinterpret_cast(request->extacl_user.rawBuf())); blen += base64_encode_update(&ctx, loginbuf+blen, 1, reinterpret_cast(":")); - blen += base64_encode_update(&ctx, loginbuf+blen, request->extacl_passwd.size(), reinterpret_cast(request->extacl_passwd.rawBuf())); + blen += base64_encode_update(&ctx, loginbuf+blen, passwdLen, reinterpret_cast(request->extacl_passwd.rawBuf())); blen += base64_encode_final(&ctx, loginbuf+blen); httpHeaderPutStrf(hdr_out, header, "Basic %.*s", (int)blen, loginbuf); return; @@ -1894,7 +1903,10 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe } #endif /* HAVE_KRB5 && HAVE_GSSAPI */ - blen = base64_encode_update(&ctx, loginbuf, strlen(request->peer_login), reinterpret_cast(request->peer_login)); + const auto loginLen = strlen(request->peer_login); + if (loginLen > MAX_LOGIN_SZ) + throw TexcHere("peer_login too long"); + blen = base64_encode_update(&ctx, loginbuf, loginLen, reinterpret_cast(request->peer_login)); blen += base64_encode_final(&ctx, loginbuf+blen); httpHeaderPutStrf(hdr_out, header, "Basic %.*s", (int)blen, loginbuf); return; diff --git a/src/tests/testExtAclBase64Overflow.cc b/src/tests/testExtAclBase64Overflow.cc index d508d1032b3..5e6168a9f6e 100644 --- a/src/tests/testExtAclBase64Overflow.cc +++ b/src/tests/testExtAclBase64Overflow.cc @@ -129,6 +129,12 @@ class TestExtAclBase64Overflow : public CPPUNIT_NS::TestFixture // Site A fix — guard condition (ModXact.cc: plainLen > MAX_LOGIN_SZ throws) CPPUNIT_TEST(testGuardAllowsExactLimit); CPPUNIT_TEST(testGuardRejectsOneByteOver); + // Site B fix — guard condition (http.cc PASS/PROXYPASS: same three-segment pattern) + CPPUNIT_TEST(testSiteB_GuardAllowsExactLimit); + CPPUNIT_TEST(testSiteB_GuardRejectsOneByteOver); + // Site C fix — guard condition (http.cc '*'-prefix: two-segment pattern) + CPPUNIT_TEST(testSiteC_GuardAllowsExactLimit); + CPPUNIT_TEST(testSiteC_GuardRejectsOneByteOver); CPPUNIT_TEST_SUITE_END(); protected: @@ -178,6 +184,24 @@ class TestExtAclBase64Overflow : public CPPUNIT_NS::TestFixture /// plainLen == MAX_LOGIN_SZ+1 (129) MUST trigger the guard. /// Without the guard this input would write 176 bytes into a 175-byte buffer. void testGuardRejectsOneByteOver(); + + // ── Site B fix: guard condition (http.cc PASS/PROXYPASS) ───────────── + // Identical three-segment encoding pattern to site A. + + /// plainLen == MAX_LOGIN_SZ (128) must NOT trigger the site B guard. + void testSiteB_GuardAllowsExactLimit(); + + /// plainLen == MAX_LOGIN_SZ+1 (129) MUST trigger the site B guard. + void testSiteB_GuardRejectsOneByteOver(); + + // ── Site C fix: guard condition (http.cc '*'-prefix) ────────────────── + // Two-segment encoding: extacl_user + peer_login_suffix (no ':'). + + /// usernameLen + suffixLen == MAX_LOGIN_SZ (128) must NOT trigger the site C guard. + void testSiteC_GuardAllowsExactLimit(); + + /// usernameLen + suffixLen == MAX_LOGIN_SZ+1 (129) MUST trigger the site C guard. + void testSiteC_GuardRejectsOneByteOver(); }; CPPUNIT_TEST_SUITE_REGISTRATION(TestExtAclBase64Overflow); @@ -381,6 +405,82 @@ TestExtAclBase64Overflow::testGuardRejectsOneByteOver() written <= ProductionBufSize); } +// ── Site B fix: http.cc PASS/PROXYPASS — identical three-segment pattern ───── + +void +TestExtAclBase64Overflow::testSiteB_GuardAllowsExactLimit() +{ + // Site B guard: if (userLen + 1 + passwdLen > MAX_LOGIN_SZ) throw + // plainLen == MAX_LOGIN_SZ (128) must NOT fire — same arithmetic as site A. + const std::string user(63, 'U'); + const std::string passwd(64, 'P'); + const auto plainLen = user.size() + 1 + passwd.size(); + CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ), plainLen); + + const auto written = encodeUserColon(user.c_str(), user.size(), + passwd.c_str(), passwd.size()); + CPPUNIT_ASSERT_MESSAGE("site B: plainLen == MAX_LOGIN_SZ must not overflow the buffer", + written <= ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testSiteB_GuardRejectsOneByteOver() +{ + // Site B guard fires at plainLen == MAX_LOGIN_SZ+1 (129). + // plainLen=129 produces exactly 175 encoded bytes — fills buffer but doesn't + // overflow; guard is conservative (fires before the 130-byte true overflow). + const std::string user(64, 'U'); + const std::string passwd(64, 'P'); + const auto plainLen = user.size() + 1 + passwd.size(); + CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ + 1), plainLen); + + CPPUNIT_ASSERT_MESSAGE("site B: plainLen == MAX_LOGIN_SZ+1 must trigger the guard", + plainLen > static_cast(MAX_LOGIN_SZ)); + + const auto written = encodeUserColon(user.c_str(), user.size(), + passwd.c_str(), passwd.size()); + CPPUNIT_ASSERT_MESSAGE("site B: plainLen == MAX_LOGIN_SZ+1 still fits in the buffer " + "(guard is conservative; actual overflow starts at plainLen=130)", + written <= ProductionBufSize); +} + +// ── Site C fix: http.cc '*'-prefix — two-segment pattern (no ':') ──────────── + +void +TestExtAclBase64Overflow::testSiteC_GuardAllowsExactLimit() +{ + // Site C guard: if (usernameLen + suffixLen > MAX_LOGIN_SZ) throw + // Combined == MAX_LOGIN_SZ (128) must NOT fire. + const std::string username(64, 'U'); + const std::string suffix(64, 'S'); + const auto plainLen = username.size() + suffix.size(); + CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ), plainLen); + + const auto written = encodeUserSuffix(username.c_str(), username.size(), + suffix.c_str(), suffix.size()); + CPPUNIT_ASSERT_MESSAGE("site C: usernameLen+suffixLen == MAX_LOGIN_SZ must not overflow", + written <= ProductionBufSize); +} + +void +TestExtAclBase64Overflow::testSiteC_GuardRejectsOneByteOver() +{ + // Site C guard fires at usernameLen + suffixLen == MAX_LOGIN_SZ+1 (129). + const std::string username(65, 'U'); + const std::string suffix(64, 'S'); + const auto plainLen = username.size() + suffix.size(); + CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ + 1), plainLen); + + CPPUNIT_ASSERT_MESSAGE("site C: usernameLen+suffixLen == MAX_LOGIN_SZ+1 must trigger the guard", + plainLen > static_cast(MAX_LOGIN_SZ)); + + const auto written = encodeUserSuffix(username.c_str(), username.size(), + suffix.c_str(), suffix.size()); + CPPUNIT_ASSERT_MESSAGE("site C: plainLen == MAX_LOGIN_SZ+1 still fits in the buffer " + "(guard is conservative; actual overflow starts at plainLen=130)", + written <= ProductionBufSize); +} + int main(int argc, char *argv[]) { From f6c053ef094353940649dca79ab267a16193466c Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sat, 20 Jun 2026 10:12:12 +0200 Subject: [PATCH 04/16] Apply aaa --- src/adaptation/icap/ModXact.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index 890ac44c255..5b78164e69e 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -1410,7 +1410,7 @@ void Adaptation::Icap::ModXact::makeRequestHeaders(MemBuf &buf) char base64buf[base64_encode_len(MAX_LOGIN_SZ)]; struct base64_encode_ctx ctx; base64_encode_init(&ctx); - size_t resultLen = base64_encode_update(&ctx, base64buf, userLen, reinterpret_cast(request->extacl_user.rawBuf())); + auto resultLen = base64_encode_update(&ctx, base64buf, userLen, reinterpret_cast(request->extacl_user.rawBuf())); resultLen += base64_encode_update(&ctx, base64buf+resultLen, 1, reinterpret_cast(":")); resultLen += base64_encode_update(&ctx, base64buf+resultLen, passwdLen, reinterpret_cast(request->extacl_passwd.rawBuf())); resultLen += base64_encode_final(&ctx, base64buf+resultLen); From ef95154a747993a80502e63e8d42920e576beaf7 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sat, 20 Jun 2026 10:17:10 +0200 Subject: [PATCH 05/16] Remove test harness --- src/Makefile.am | 21 -- src/tests/testExtAclBase64Overflow.cc | 488 -------------------------- 2 files changed, 509 deletions(-) delete mode 100644 src/tests/testExtAclBase64Overflow.cc diff --git a/src/Makefile.am b/src/Makefile.am index 966f45da0c5..4480764023d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1994,27 +1994,6 @@ tests_testHttpRequest_LDADD = \ $(XTRA_LIBS) tests_testHttpRequest_LDFLAGS = $(LIBADD_DL) -## Characterization test: extacl base64 stack buffer overflow (CVE candidate) -## Confirms that extacl_user/extacl_passwd values exceeding MAX_LOGIN_SZ -## cause base64 output to overflow the fixed-size stack buffers in -## ModXact.cc::makeRequestHeaders() and http.cc::httpFixupAuthentication(). - -check_PROGRAMS += tests/testExtAclBase64Overflow -tests_testExtAclBase64Overflow_SOURCES = \ - tests/testExtAclBase64Overflow.cc -nodist_tests_testExtAclBase64Overflow_SOURCES = \ - $(TESTSOURCES) \ - tests/stub_debug.cc \ - tests/stub_libmem.cc -tests_testExtAclBase64Overflow_LDADD = \ - $(top_builddir)/lib/libmiscencoding.la \ - base/libbase.la \ - $(LIBCPPUNIT_LIBS) \ - $(COMPAT_LIB) \ - $(LIBNETTLE_LIBS) \ - $(XTRA_LIBS) -tests_testExtAclBase64Overflow_LDFLAGS = $(LIBADD_DL) - ## Tests of ip/* check_PROGRAMS += tests/testIpAddress diff --git a/src/tests/testExtAclBase64Overflow.cc b/src/tests/testExtAclBase64Overflow.cc deleted file mode 100644 index 5e6168a9f6e..00000000000 --- a/src/tests/testExtAclBase64Overflow.cc +++ /dev/null @@ -1,488 +0,0 @@ -/* - * Copyright (C) 1996-2026 The Squid Software Foundation and contributors - * - * Squid software is distributed under GPLv2+ license and includes - * contributions from numerous individuals and organizations. - * Please see the COPYING and CONTRIBUTORS files for details. - */ - -/* - * Characterization and fix-verification tests for the stack buffer overflow - * in the external-ACL credential base64 encoding paths. - * - * Three production sites allocate a fixed-size stack buffer sized for - * MAX_LOGIN_SZ bytes of pre-encoding input: - * - * src/adaptation/icap/ModXact.cc (makeRequestHeaders) — site A - * src/http.cc httpFixupAuthentication() PASS/PROXYPASS branch — site B - * src/http.cc httpFixupAuthentication() '*'-prefix branch — site C - * - * Sites A and B encode: extacl_user + ":" + extacl_passwd - * Site C encodes: extacl_user + peer_login_suffix - * - * Without a guard, all three pass the actual (unchecked) field sizes - * directly to base64_encode_update() into a 175-byte stack buffer. - * When a helper returns values whose combined plain-text length exceeds - * 129 bytes the encoded output (>= 176 bytes) overflows the buffer. - * - * The fix for site A (ModXact.cc) adds a pre-encoding guard: - * - * if (plainLen > MAX_LOGIN_SZ) // i.e. > 128 - * throw TexcHere(...); - * - * The buffer-overflow section of this file characterizes the encoding - * arithmetic (showing the overflow would occur without the guard). - * The guard-condition section verifies the fix's threshold directly. - * - * Buffer overflow threshold (encoding arithmetic): - * At 130 bytes of plain input the encoded output is 176 bytes, - * which exceeds the 175-byte buffer by 1 byte. - * - * Guard threshold (site A fix): - * plainLen > MAX_LOGIN_SZ (128) triggers the throw. - * plainLen == 128 is safe; plainLen == 129 is rejected. - */ - -#include "squid.h" -#include "compat/cppunit.h" -#include "defines.h" -#include "unitTestMain.h" - -#include "base64.h" - -#include -#include -#include -#include - -/// The fixed-size stack buffer used in all three vulnerable sites. -/// base64_encode_len(128) = ((128*8+4)/6) + 3 + 1 = 171 + 3 + 1 = 175. -static constexpr size_t ProductionBufSize = base64_encode_len(MAX_LOGIN_SZ); - -/// Replicates the encoding pattern used in: -/// - ModXact.cc::makeRequestHeaders() -/// - httpFixupAuthentication() PASS/PROXYPASS branch (sites A and B) -/// -/// Encodes: user + ":" + passwd (three-segment pattern) -static size_t -encodeUserColon(const char *user, size_t userLen, - const char *passwd, size_t passwdLen) -{ - // Scratch buffer large enough for any input we test — avoids UB while - // faithfully counting emitted bytes. - std::vector scratch(8192, '\0'); - char *dst = scratch.data(); - - struct base64_encode_ctx ctx; - base64_encode_init(&ctx); - - size_t total = 0; - total += base64_encode_update(&ctx, dst + total, userLen, - reinterpret_cast(user)); - total += base64_encode_update(&ctx, dst + total, 1, - reinterpret_cast(":")); - total += base64_encode_update(&ctx, dst + total, passwdLen, - reinterpret_cast(passwd)); - total += base64_encode_final(&ctx, dst + total); - return total; -} - -/// Replicates the encoding pattern used in: -/// - httpFixupAuthentication() '*'-prefix branch (site C) -/// -/// Encodes: username + peerLoginSuffix (two-segment pattern, no ':') -static size_t -encodeUserSuffix(const char *username, size_t usernameLen, - const char *suffix, size_t suffixLen) -{ - std::vector scratch(8192, '\0'); - char *dst = scratch.data(); - - struct base64_encode_ctx ctx; - base64_encode_init(&ctx); - - size_t total = 0; - total += base64_encode_update(&ctx, dst + total, usernameLen, - reinterpret_cast(username)); - total += base64_encode_update(&ctx, dst + total, suffixLen, - reinterpret_cast(suffix)); - total += base64_encode_final(&ctx, dst + total); - return total; -} - -class TestExtAclBase64Overflow : public CPPUNIT_NS::TestFixture -{ - CPPUNIT_TEST_SUITE(TestExtAclBase64Overflow); - // Buffer-size anchor - CPPUNIT_TEST(testBufferSizeCalculation); - // Sites A & B — user + ":" + passwd pattern - CPPUNIT_TEST(testSafeInputFitsBuffer); - CPPUNIT_TEST(testMaxSafeBoundaryFits); - CPPUNIT_TEST(testExactOverflowThreshold); - CPPUNIT_TEST(testOversizedUserOverflows); - CPPUNIT_TEST(testOversizedPasswdOverflows); - CPPUNIT_TEST(testCombinedOverflow); - CPPUNIT_TEST(testLargeInputOverflows); - // Site C — user + peer_login_suffix pattern - CPPUNIT_TEST(testSiteC_SafeInputFitsBuffer); - CPPUNIT_TEST(testSiteC_OversizedUserOverflows); - // Site A fix — guard condition (ModXact.cc: plainLen > MAX_LOGIN_SZ throws) - CPPUNIT_TEST(testGuardAllowsExactLimit); - CPPUNIT_TEST(testGuardRejectsOneByteOver); - // Site B fix — guard condition (http.cc PASS/PROXYPASS: same three-segment pattern) - CPPUNIT_TEST(testSiteB_GuardAllowsExactLimit); - CPPUNIT_TEST(testSiteB_GuardRejectsOneByteOver); - // Site C fix — guard condition (http.cc '*'-prefix: two-segment pattern) - CPPUNIT_TEST(testSiteC_GuardAllowsExactLimit); - CPPUNIT_TEST(testSiteC_GuardRejectsOneByteOver); - CPPUNIT_TEST_SUITE_END(); - -protected: - /// Confirm the production buffer is exactly 175 bytes and MAX_LOGIN_SZ is 128. - void testBufferSizeCalculation(); - - // ── Sites A & B: user + ":" + passwd ───────────────────────────────── - - /// Well-within-limit input does not overflow. - void testSafeInputFitsBuffer(); - - /// user=63, passwd=64: combined plain=128 bytes — fits with 3 bytes spare. - void testMaxSafeBoundaryFits(); - - /// user=64, passwd=65: combined plain=130 bytes — the minimal overflow. - /// base64(130 bytes) = 176 bytes > 175-byte buffer. - void testExactOverflowThreshold(); - - /// user alone exceeds MAX_LOGIN_SZ — sites A and B overflow. - void testOversizedUserOverflows(); - - /// passwd alone exceeds MAX_LOGIN_SZ — sites A and B overflow. - void testOversizedPasswdOverflows(); - - /// Moderate combined excess (65+64=129+":"=130) — both sites overflow. - void testCombinedOverflow(); - - /// Very large input — confirms no arithmetic wrapping masks the overflow. - void testLargeInputOverflows(); - - // ── Site C: user + peer_login_suffix ───────────────────────────────── - - /// Small user + small suffix stays within buffer. - void testSiteC_SafeInputFitsBuffer(); - - /// Oversized extacl_user overflows the same loginbuf via site C's path. - void testSiteC_OversizedUserOverflows(); - - // ── Site A fix: guard condition verification ────────────────────────── - // The fix in ModXact.cc guards: if (plainLen > MAX_LOGIN_SZ) throw ... - // These tests verify the arithmetic of that guard directly. - - /// plainLen == MAX_LOGIN_SZ (128) must NOT trigger the guard. - /// Encodes correctly within the 175-byte buffer (172 bytes written). - void testGuardAllowsExactLimit(); - - /// plainLen == MAX_LOGIN_SZ+1 (129) MUST trigger the guard. - /// Without the guard this input would write 176 bytes into a 175-byte buffer. - void testGuardRejectsOneByteOver(); - - // ── Site B fix: guard condition (http.cc PASS/PROXYPASS) ───────────── - // Identical three-segment encoding pattern to site A. - - /// plainLen == MAX_LOGIN_SZ (128) must NOT trigger the site B guard. - void testSiteB_GuardAllowsExactLimit(); - - /// plainLen == MAX_LOGIN_SZ+1 (129) MUST trigger the site B guard. - void testSiteB_GuardRejectsOneByteOver(); - - // ── Site C fix: guard condition (http.cc '*'-prefix) ────────────────── - // Two-segment encoding: extacl_user + peer_login_suffix (no ':'). - - /// usernameLen + suffixLen == MAX_LOGIN_SZ (128) must NOT trigger the site C guard. - void testSiteC_GuardAllowsExactLimit(); - - /// usernameLen + suffixLen == MAX_LOGIN_SZ+1 (129) MUST trigger the site C guard. - void testSiteC_GuardRejectsOneByteOver(); -}; - -CPPUNIT_TEST_SUITE_REGISTRATION(TestExtAclBase64Overflow); - -void -TestExtAclBase64Overflow::testBufferSizeCalculation() -{ - // base64_encode_len(128): - // BASE64_ENCODE_LENGTH(128) = (128×8 + 4) / 6 = 1028/6 = 171 (integer division) - // BASE64_ENCODE_FINAL_LENGTH = 3 - // NUL sentinel = 1 - // = 175 - CPPUNIT_ASSERT_EQUAL(static_cast(175), ProductionBufSize); - CPPUNIT_ASSERT_EQUAL(static_cast(128), static_cast(MAX_LOGIN_SZ)); -} - -void -TestExtAclBase64Overflow::testSafeInputFitsBuffer() -{ - // user="alice" (5), passwd="secret" (6): total plain = 12 bytes — well under 128. - const char *user = "alice"; - const char *passwd = "secret"; - const auto written = encodeUserColon(user, strlen(user), passwd, strlen(passwd)); - - CPPUNIT_ASSERT_MESSAGE("safe credentials must fit in the fixed stack buffer", - written <= ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testMaxSafeBoundaryFits() -{ - // user=63 + ":" + passwd=64 = 128 bytes total — exactly MAX_LOGIN_SZ. - // BASE64_ENCODE_RAW_LENGTH(128) = ((128+2)/3)*4 = 43*4 = 172 bytes written. - // 172 <= 175: fits with 3 bytes to spare. - const std::string user(63, 'U'); - const std::string passwd(64, 'P'); - const auto written = encodeUserColon(user.c_str(), user.size(), - passwd.c_str(), passwd.size()); - - CPPUNIT_ASSERT_MESSAGE("input of exactly MAX_LOGIN_SZ bytes must fit in fixed buffer", - written <= ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testExactOverflowThreshold() -{ - // user=64 + ":" + passwd=65 = 130 bytes total — the minimal overflowing input. - // BASE64_ENCODE_RAW_LENGTH(130) = ((130+2)/3)*4 = 44*4 = 176 bytes. - // 176 > 175: the first input size that actually overflows the buffer. - const std::string user(64, 'U'); - const std::string passwd(65, 'P'); - const auto written = encodeUserColon(user.c_str(), user.size(), - passwd.c_str(), passwd.size()); - - CPPUNIT_ASSERT_MESSAGE("130-byte combined input (user+colon+passwd) is the minimal overflow", - written > ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testOversizedUserOverflows() -{ - // user=129 bytes alone (+ ":" + empty passwd = 130 bytes total). - // The user field exceeds MAX_LOGIN_SZ; production code at sites A and B - // passes user.size() = 129 directly to base64_encode_update() without - // checking, writing into a 175-byte stack buffer. - // Total plain = 130 bytes => 176 bytes encoded > 175 bytes available. - const std::string user(129, 'A'); - const char *passwd = ""; - const auto written = encodeUserColon(user.c_str(), user.size(), passwd, strlen(passwd)); - - CPPUNIT_ASSERT_MESSAGE("oversized user field (site A: ModXact, site B: httpFixupAuthentication) " - "causes encoded output to exceed fixed stack buffer", - written > ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testOversizedPasswdOverflows() -{ - // user="u" (1 byte) + ":" + passwd=129 bytes = 131 bytes total. - // passwd alone exceeds MAX_LOGIN_SZ; passed unchecked at sites A and B. - // 131 bytes encoded = 176 bytes > 175 bytes. - const char *user = "u"; - const std::string passwd(129, 'P'); - const auto written = encodeUserColon(user, strlen(user), passwd.c_str(), passwd.size()); - - CPPUNIT_ASSERT_MESSAGE("oversized passwd field (site A: ModXact, site B: httpFixupAuthentication) " - "causes encoded output to exceed fixed stack buffer", - written > ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testCombinedOverflow() -{ - // user=65 + ":" + passwd=64 = 130 bytes total. - // Each field looks reasonable in isolation (both < MAX_LOGIN_SZ) but - // their combination exceeds the safe threshold by 2 bytes. - const std::string user(65, 'U'); - const std::string passwd(64, 'P'); - const auto written = encodeUserColon(user.c_str(), user.size(), - passwd.c_str(), passwd.size()); - - CPPUNIT_ASSERT_MESSAGE("combined user+colon+passwd exceeding safe threshold " - "overflows fixed stack buffer", - written > ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testLargeInputOverflows() -{ - // 512+512 = 1024 bytes; confirms no size-type wrapping masks the overflow. - const std::string user(512, 'A'); - const std::string passwd(512, 'B'); - const auto written = encodeUserColon(user.c_str(), user.size(), - passwd.c_str(), passwd.size()); - - CPPUNIT_ASSERT_MESSAGE("very large input must also overflow fixed stack buffer", - written > ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testSiteC_SafeInputFitsBuffer() -{ - // Site C (httpFixupAuthentication '*'-prefix branch) encodes: - // extacl_user + peer_login_suffix (no ':') - // into the same loginbuf[base64_encode_len(MAX_LOGIN_SZ)] = 175 bytes. - // Small inputs fit fine. - const char *username = "alice"; - const char *suffix = "@realm"; - const auto written = encodeUserSuffix(username, strlen(username), - suffix, strlen(suffix)); - - CPPUNIT_ASSERT_MESSAGE("site C: small extacl_user + peer_login_suffix must fit in fixed buffer", - written <= ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testSiteC_OversizedUserOverflows() -{ - // Site C: extacl_user = 129 bytes + any non-empty peer_login_suffix. - // Total plain = 130 bytes => 176 bytes encoded > 175-byte loginbuf. - const std::string username(129, 'U'); - const char *suffix = "x"; // minimal non-empty suffix - const auto written = encodeUserSuffix(username.c_str(), username.size(), - suffix, strlen(suffix)); - - CPPUNIT_ASSERT_MESSAGE("site C: oversized extacl_user causes encoded output to exceed " - "fixed loginbuf in httpFixupAuthentication '*'-prefix path", - written > ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testGuardAllowsExactLimit() -{ - // The site A guard condition is: if (plainLen > MAX_LOGIN_SZ) throw - // plainLen == MAX_LOGIN_SZ (128) must NOT trigger it. - // - // user=63 + ":" + passwd=64 = 128 bytes. - // BASE64_ENCODE_RAW_LENGTH(128) = 172 bytes written; 172 <= 175. Safe. - const std::string user(63, 'U'); - const std::string passwd(64, 'P'); - const auto plainLen = user.size() + 1 + passwd.size(); - CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ), plainLen); - - const auto written = encodeUserColon(user.c_str(), user.size(), - passwd.c_str(), passwd.size()); - CPPUNIT_ASSERT_MESSAGE("plainLen == MAX_LOGIN_SZ must not overflow the buffer " - "(guard must allow this input)", - written <= ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testGuardRejectsOneByteOver() -{ - // The site A guard condition is: if (plainLen > MAX_LOGIN_SZ) throw - // plainLen == MAX_LOGIN_SZ+1 (129) MUST trigger the guard. - // - // Note: the guard is deliberately conservative. At plainLen=129 the - // encoded output is exactly 175 bytes (== ProductionBufSize), which - // technically fits. However the guard fires at plainLen > MAX_LOGIN_SZ, - // rejecting 129 to keep a safe margin for the 3-byte final padding slot. - // The true minimal overflowing input is 130 bytes (see testExactOverflowThreshold). - // - // This test confirms: - // (a) plainLen == MAX_LOGIN_SZ + 1 (inputs constructed correctly) - // (b) the guard condition (plainLen > MAX_LOGIN_SZ) is TRUE - // (c) the encoded output still fits (guard fires before actual overflow) - const std::string user(64, 'U'); - const std::string passwd(64, 'P'); - const auto plainLen = user.size() + 1 + passwd.size(); - CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ + 1), plainLen); - - // (b) guard fires: plainLen > MAX_LOGIN_SZ - CPPUNIT_ASSERT_MESSAGE("plainLen == MAX_LOGIN_SZ+1 must trigger the guard condition", - plainLen > static_cast(MAX_LOGIN_SZ)); - - // (c) at this size the encoder writes exactly ProductionBufSize bytes (no margin) - const auto written = encodeUserColon(user.c_str(), user.size(), - passwd.c_str(), passwd.size()); - CPPUNIT_ASSERT_MESSAGE("plainLen == MAX_LOGIN_SZ+1 still fits in the buffer " - "(guard is conservative; actual overflow starts at plainLen=130)", - written <= ProductionBufSize); -} - -// ── Site B fix: http.cc PASS/PROXYPASS — identical three-segment pattern ───── - -void -TestExtAclBase64Overflow::testSiteB_GuardAllowsExactLimit() -{ - // Site B guard: if (userLen + 1 + passwdLen > MAX_LOGIN_SZ) throw - // plainLen == MAX_LOGIN_SZ (128) must NOT fire — same arithmetic as site A. - const std::string user(63, 'U'); - const std::string passwd(64, 'P'); - const auto plainLen = user.size() + 1 + passwd.size(); - CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ), plainLen); - - const auto written = encodeUserColon(user.c_str(), user.size(), - passwd.c_str(), passwd.size()); - CPPUNIT_ASSERT_MESSAGE("site B: plainLen == MAX_LOGIN_SZ must not overflow the buffer", - written <= ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testSiteB_GuardRejectsOneByteOver() -{ - // Site B guard fires at plainLen == MAX_LOGIN_SZ+1 (129). - // plainLen=129 produces exactly 175 encoded bytes — fills buffer but doesn't - // overflow; guard is conservative (fires before the 130-byte true overflow). - const std::string user(64, 'U'); - const std::string passwd(64, 'P'); - const auto plainLen = user.size() + 1 + passwd.size(); - CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ + 1), plainLen); - - CPPUNIT_ASSERT_MESSAGE("site B: plainLen == MAX_LOGIN_SZ+1 must trigger the guard", - plainLen > static_cast(MAX_LOGIN_SZ)); - - const auto written = encodeUserColon(user.c_str(), user.size(), - passwd.c_str(), passwd.size()); - CPPUNIT_ASSERT_MESSAGE("site B: plainLen == MAX_LOGIN_SZ+1 still fits in the buffer " - "(guard is conservative; actual overflow starts at plainLen=130)", - written <= ProductionBufSize); -} - -// ── Site C fix: http.cc '*'-prefix — two-segment pattern (no ':') ──────────── - -void -TestExtAclBase64Overflow::testSiteC_GuardAllowsExactLimit() -{ - // Site C guard: if (usernameLen + suffixLen > MAX_LOGIN_SZ) throw - // Combined == MAX_LOGIN_SZ (128) must NOT fire. - const std::string username(64, 'U'); - const std::string suffix(64, 'S'); - const auto plainLen = username.size() + suffix.size(); - CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ), plainLen); - - const auto written = encodeUserSuffix(username.c_str(), username.size(), - suffix.c_str(), suffix.size()); - CPPUNIT_ASSERT_MESSAGE("site C: usernameLen+suffixLen == MAX_LOGIN_SZ must not overflow", - written <= ProductionBufSize); -} - -void -TestExtAclBase64Overflow::testSiteC_GuardRejectsOneByteOver() -{ - // Site C guard fires at usernameLen + suffixLen == MAX_LOGIN_SZ+1 (129). - const std::string username(65, 'U'); - const std::string suffix(64, 'S'); - const auto plainLen = username.size() + suffix.size(); - CPPUNIT_ASSERT_EQUAL(static_cast(MAX_LOGIN_SZ + 1), plainLen); - - CPPUNIT_ASSERT_MESSAGE("site C: usernameLen+suffixLen == MAX_LOGIN_SZ+1 must trigger the guard", - plainLen > static_cast(MAX_LOGIN_SZ)); - - const auto written = encodeUserSuffix(username.c_str(), username.size(), - suffix.c_str(), suffix.size()); - CPPUNIT_ASSERT_MESSAGE("site C: plainLen == MAX_LOGIN_SZ+1 still fits in the buffer " - "(guard is conservative; actual overflow starts at plainLen=130)", - written <= ProductionBufSize); -} - -int -main(int argc, char *argv[]) -{ - return TestProgram().run(argc, argv); -} From d7fa149500af1b17fd8e7a8ae147115bacaae0e7 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sat, 20 Jun 2026 22:55:52 +0200 Subject: [PATCH 06/16] Use TextException --- src/adaptation/icap/ModXact.cc | 2 +- src/http.cc | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index 5b78164e69e..b20685455fd 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -1404,7 +1404,7 @@ void Adaptation::Icap::ModXact::makeRequestHeaders(MemBuf &buf) // +1 for the ':' separator between user and passwd const auto plainLen = userLen + 1 + passwdLen; if (plainLen > MAX_LOGIN_SZ) - throw TexcHere("extacl credentials too long for Proxy-Authorization"); + throw TextException("extacl credentials too long for Proxy-Authorization", Here()); // plainLen <= MAX_LOGIN_SZ, so base64_encode_len(plainLen) fits // within the base64_encode_len(MAX_LOGIN_SZ) stack buffer. char base64buf[base64_encode_len(MAX_LOGIN_SZ)]; diff --git a/src/http.cc b/src/http.cc index de941d451b3..400a80c27ab 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1853,7 +1853,7 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe const auto usernameLen = strlen(username); const auto suffixLen = strlen(request->peer_login + 1); if (usernameLen + suffixLen > MAX_LOGIN_SZ) - throw TexcHere("peer login credentials too long"); + throw TextException("peer login credentials too long", Here()); blen = base64_encode_update(&ctx, loginbuf, usernameLen, reinterpret_cast(username)); blen += base64_encode_update(&ctx, loginbuf+blen, suffixLen, reinterpret_cast(request->peer_login +1)); blen += base64_encode_final(&ctx, loginbuf+blen); @@ -1870,7 +1870,7 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe const auto passwdLen = request->extacl_passwd.size(); // +1 for the ':' separator between user and passwd if (userLen + 1 + passwdLen > MAX_LOGIN_SZ) - throw TexcHere("extacl credentials too long for peer login"); + throw TextException("extacl credentials too long for peer login", Here()); blen = base64_encode_update(&ctx, loginbuf, userLen, reinterpret_cast(request->extacl_user.rawBuf())); blen += base64_encode_update(&ctx, loginbuf+blen, 1, reinterpret_cast(":")); blen += base64_encode_update(&ctx, loginbuf+blen, passwdLen, reinterpret_cast(request->extacl_passwd.rawBuf())); @@ -1905,7 +1905,7 @@ httpFixupAuthentication(HttpRequest * request, const HttpHeader * hdr_in, HttpHe const auto loginLen = strlen(request->peer_login); if (loginLen > MAX_LOGIN_SZ) - throw TexcHere("peer_login too long"); + throw TextException("peer_login too long", Here()); blen = base64_encode_update(&ctx, loginbuf, loginLen, reinterpret_cast(request->peer_login)); blen += base64_encode_final(&ctx, loginbuf+blen); httpHeaderPutStrf(hdr_out, header, "Basic %.*s", (int)blen, loginbuf); From 5044dd7c958850a3ba0b5570b57ff33bca856779 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli <5175948+kinkie@users.noreply.github.com> Date: Sun, 5 Jul 2026 23:25:47 +0100 Subject: [PATCH 07/16] Update src/adaptation/icap/ModXact.cc Co-authored-by: Alex Rousskov --- src/adaptation/icap/ModXact.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index b20685455fd..3ad64b8f40e 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -1405,8 +1405,6 @@ void Adaptation::Icap::ModXact::makeRequestHeaders(MemBuf &buf) const auto plainLen = userLen + 1 + passwdLen; if (plainLen > MAX_LOGIN_SZ) throw TextException("extacl credentials too long for Proxy-Authorization", Here()); - // plainLen <= MAX_LOGIN_SZ, so base64_encode_len(plainLen) fits - // within the base64_encode_len(MAX_LOGIN_SZ) stack buffer. char base64buf[base64_encode_len(MAX_LOGIN_SZ)]; struct base64_encode_ctx ctx; base64_encode_init(&ctx); From c037cd42c98b04648ae4a7c2bf1ac6274468e82c Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Tue, 7 Jul 2026 22:05:18 +0100 Subject: [PATCH 08/16] define -> constexpr auto --- src/http.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http.cc b/src/http.cc index 400a80c27ab..a83e7d38eb8 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1926,7 +1926,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, const Http::StateFlags &flags) { /* building buffer for complex strings */ -#define BBUF_SZ (MAX_URL+32) + constexpr auto BBUF_SZ = MAX_URL + 32; LOCAL_ARRAY(char, bbuf, BBUF_SZ); LOCAL_ARRAY(char, ntoabuf, MAX_IPSTRLEN); const HttpHeader *hdr_in = &request->header; From 8503055650ef1531b20940a71525f53dbf58abac Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Tue, 7 Jul 2026 23:23:51 +0100 Subject: [PATCH 09/16] Add field size checks --- src/http.cc | 1 + src/peer_proxy_negotiate_auth.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/src/http.cc b/src/http.cc index a83e7d38eb8..7f8e46ecea8 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1933,6 +1933,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, const HttpHeaderEntry *e = nullptr; HttpHeaderPos pos = HttpHeaderInitPos; assert (hdr_out->owner == hoRequest); + Assure(request->url.userInfo().length() < MAX_URL*2); /* use our IMS header if the cached entry has Last-Modified time */ if (request->lastmod > -1) diff --git a/src/peer_proxy_negotiate_auth.cc b/src/peer_proxy_negotiate_auth.cc index eb937e5ec38..b264737cf07 100644 --- a/src/peer_proxy_negotiate_auth.cc +++ b/src/peer_proxy_negotiate_auth.cc @@ -546,6 +546,7 @@ char *peer_proxy_negotiate_auth(char *principal_name, const char * const proxy, static char b64buf[8192]; // XXX: 8KB only because base64_encode_bin() used to. struct base64_encode_ctx ctx; base64_encode_init(&ctx); + Assure(output_token.length < sizeof(b64buf)); size_t blen = base64_encode_update(&ctx, b64buf, output_token.length, reinterpret_cast(output_token.value)); blen += base64_encode_final(&ctx, b64buf+blen); b64buf[blen] = '\0'; From 8dff6039038778f426c05e881ecd1cb3902dc7f9 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sat, 11 Jul 2026 23:16:17 +0100 Subject: [PATCH 10/16] Fix Assure() position and include header --- src/http.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/http.cc b/src/http.cc index 7f8e46ecea8..05d50c3f7a1 100644 --- a/src/http.cc +++ b/src/http.cc @@ -15,6 +15,7 @@ #include "squid.h" #include "acl/FilledChecklist.h" +#include "base/Assure.h" #include "base/AsyncJobCalls.h" #include "base/DelayedAsyncCalls.h" #include "base/Raw.h" @@ -1933,7 +1934,6 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, const HttpHeaderEntry *e = nullptr; HttpHeaderPos pos = HttpHeaderInitPos; assert (hdr_out->owner == hoRequest); - Assure(request->url.userInfo().length() < MAX_URL*2); /* use our IMS header if the cached entry has Last-Modified time */ if (request->lastmod > -1) @@ -2034,6 +2034,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, static char result[base64_encode_len(MAX_URL*2)]; // should be big enough for a single URI segment struct base64_encode_ctx ctx; base64_encode_init(&ctx); + Assure(request->url.userInfo().length() < MAX_URL*2); size_t blen = base64_encode_update(&ctx, result, request->url.userInfo().length(), reinterpret_cast(request->url.userInfo().rawContent())); blen += base64_encode_final(&ctx, result+blen); result[blen] = '\0'; From da6bc4aa6f2ecd5e36d0182a03f792d7ea3adb5e Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sat, 11 Jul 2026 23:50:52 +0100 Subject: [PATCH 11/16] Revert broken changes to ModXact.cc --- src/http.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/http.cc b/src/http.cc index 05d50c3f7a1..7f8e46ecea8 100644 --- a/src/http.cc +++ b/src/http.cc @@ -15,7 +15,6 @@ #include "squid.h" #include "acl/FilledChecklist.h" -#include "base/Assure.h" #include "base/AsyncJobCalls.h" #include "base/DelayedAsyncCalls.h" #include "base/Raw.h" @@ -1934,6 +1933,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, const HttpHeaderEntry *e = nullptr; HttpHeaderPos pos = HttpHeaderInitPos; assert (hdr_out->owner == hoRequest); + Assure(request->url.userInfo().length() < MAX_URL*2); /* use our IMS header if the cached entry has Last-Modified time */ if (request->lastmod > -1) @@ -2034,7 +2034,6 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, static char result[base64_encode_len(MAX_URL*2)]; // should be big enough for a single URI segment struct base64_encode_ctx ctx; base64_encode_init(&ctx); - Assure(request->url.userInfo().length() < MAX_URL*2); size_t blen = base64_encode_update(&ctx, result, request->url.userInfo().length(), reinterpret_cast(request->url.userInfo().rawContent())); blen += base64_encode_final(&ctx, result+blen); result[blen] = '\0'; From 987250ed436944ddae7532fc985f69683de7f8c3 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sun, 12 Jul 2026 00:22:12 +0100 Subject: [PATCH 12/16] add missing include --- src/peer_proxy_negotiate_auth.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/peer_proxy_negotiate_auth.cc b/src/peer_proxy_negotiate_auth.cc index b264737cf07..dd352cd6780 100644 --- a/src/peer_proxy_negotiate_auth.cc +++ b/src/peer_proxy_negotiate_auth.cc @@ -13,6 +13,7 @@ #include "squid.h" #if HAVE_AUTH_MODULE_NEGOTIATE && HAVE_KRB5 && HAVE_GSSAPI +#include "base/Assure.h" #include "base64.h" #include "compat/krb5.h" #include "debug/Stream.h" From eb5e5335527b7e78c9f456f57cbf9de34c5f7fa9 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sun, 12 Jul 2026 00:24:08 +0100 Subject: [PATCH 13/16] Accept review suggestion --- src/peer_proxy_negotiate_auth.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/peer_proxy_negotiate_auth.cc b/src/peer_proxy_negotiate_auth.cc index dd352cd6780..c0a44f709cf 100644 --- a/src/peer_proxy_negotiate_auth.cc +++ b/src/peer_proxy_negotiate_auth.cc @@ -547,7 +547,7 @@ char *peer_proxy_negotiate_auth(char *principal_name, const char * const proxy, static char b64buf[8192]; // XXX: 8KB only because base64_encode_bin() used to. struct base64_encode_ctx ctx; base64_encode_init(&ctx); - Assure(output_token.length < sizeof(b64buf)); + Assure(base64_encode_len(output_token.length) < sizeof(b64buf)); size_t blen = base64_encode_update(&ctx, b64buf, output_token.length, reinterpret_cast(output_token.value)); blen += base64_encode_final(&ctx, b64buf+blen); b64buf[blen] = '\0'; From 8305b74fa55f9ffd1265eea46be13fdf70c778b0 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sun, 12 Jul 2026 00:40:36 +0100 Subject: [PATCH 14/16] add size guard to SSP_MakeChallenge --- lib/sspi/sspwin32.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/sspi/sspwin32.cc b/lib/sspi/sspwin32.cc index 636731751bb..a07b5ceb301 100644 --- a/lib/sspi/sspwin32.cc +++ b/lib/sspi/sspwin32.cc @@ -500,6 +500,7 @@ const char * WINAPI SSP_MakeChallenge(PVOID PNegotiateBuf, int NegotiateLen) struct base64_encode_ctx ctx; base64_encode_init(&ctx); static char encoded[8192]; + assert(base64_encode_len(cbOut) < sizeof(encoded)); size_t dstLen = base64_encode_update(&ctx, encoded, cbOut, reinterpret_cast(fResult)); assert(dstLen < sizeof(encoded)); dstLen += base64_encode_final(&ctx, encoded+dstLen); From bc1955fb7a46fe931f9c761d25329e292110d245 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sun, 12 Jul 2026 00:44:21 +0100 Subject: [PATCH 15/16] revert BUF_SZ change --- src/http.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http.cc b/src/http.cc index 7f8e46ecea8..992fb8c7899 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1926,7 +1926,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, const Http::StateFlags &flags) { /* building buffer for complex strings */ - constexpr auto BBUF_SZ = MAX_URL + 32; +#define BBUF_SZ (MAX_URL+32) LOCAL_ARRAY(char, bbuf, BBUF_SZ); LOCAL_ARRAY(char, ntoabuf, MAX_IPSTRLEN); const HttpHeader *hdr_in = &request->header; From ef7196241bece1056058e11dd1d7f3a3589a66d0 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Sun, 19 Jul 2026 22:22:24 +0100 Subject: [PATCH 16/16] Move Assure call to its proper place --- src/http.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http.cc b/src/http.cc index 992fb8c7899..c9e48dfaedf 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1933,7 +1933,6 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, const HttpHeaderEntry *e = nullptr; HttpHeaderPos pos = HttpHeaderInitPos; assert (hdr_out->owner == hoRequest); - Assure(request->url.userInfo().length() < MAX_URL*2); /* use our IMS header if the cached entry has Last-Modified time */ if (request->lastmod > -1) @@ -2031,6 +2030,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request, /* append Authorization if known in URL, not in header and going direct */ if (!hdr_out->has(Http::HdrType::AUTHORIZATION)) { if (flags.toOrigin && !request->url.userInfo().isEmpty()) { + Assure(request->url.userInfo().length() < MAX_URL*2); static char result[base64_encode_len(MAX_URL*2)]; // should be big enough for a single URI segment struct base64_encode_ctx ctx; base64_encode_init(&ctx);