From c402339bbc301ad8ee27132c18bb31c3bf3f9ed5 Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Mon, 15 Jun 2026 23:45:02 +0100 Subject: [PATCH 1/6] 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 2/6] 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 3/6] 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 4/6] 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 5/6] 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 6/6] 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);