From 5f0585ef767b61d8cd955e068064f6b4a25a77d4 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Fri, 3 Apr 2026 14:58:20 -0400 Subject: [PATCH] squid: 5-MOAD scan complete; add TICKET.md for squid-0001/0002/0003, mark SCAN-TODO done All 4 squid defects confirmed: squid-0001 CWE-407 NotePairs::appendNewOnly hasPair O(S*D) 374x; squid-0002 CWE-407 removeConnectionHeaderEntries strListIsMember O(H*C) 3x; squid-0003 CWE-312 FTP+Basic auth passwords logged verbatim incl DBG_IMPORTANT; squid-0004 CWE-407 whichPeer() O(P*A) ICP peer map 83x. MOAD-0002 SquidConfig god-object documented (structural). MOAD-0003/0005 CLEAN. All 4 unit tests PASS. --- SCAN-TODO.md | 2 +- defects/squid-0001/TICKET.md | 43 ++++++++++++++++++++++++++++ defects/squid-0002/TICKET.md | 41 +++++++++++++++++++++++++++ defects/squid-0003/TICKET.md | 54 ++++++++++++++++++++++++++++++++++++ 4 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 defects/squid-0001/TICKET.md create mode 100644 defects/squid-0002/TICKET.md create mode 100644 defects/squid-0003/TICKET.md diff --git a/SCAN-TODO.md b/SCAN-TODO.md index 07c92be3c..756c0c268 100644 --- a/SCAN-TODO.md +++ b/SCAN-TODO.md @@ -5,7 +5,7 @@ Rule: clone, scan, delete clone after. Keep disk under 90%. ## Priority 1 — Major infrastructure not yet scanned -- [ ] Squid (C, HTTP proxy, huge install base) +- [x] Squid (C, HTTP proxy, huge install base) — squid-0001 MOAD-0001 CWE-407 NotePairs::appendNewOnly hasPair() O(S*E) 374x; squid-0002 MOAD-0001 CWE-407 removeConnectionHeaderEntries strListIsMember O(H*C) 3x; squid-0003 MOAD-0004 CWE-312 FTP+Basic auth credentials logged verbatim incl DBG_IMPORTANT; squid-0004 MOAD-0001 CWE-407 whichPeer() O(P*A) ICP peer lookup 83x at P=100; MOAD-0002 SquidConfig god-object 571 lines/209 files (structural, not patchable); MOAD-0003/0005 CLEAN - [x] Sendmail (C, MTA) — sendmail-0001 MOAD-0004 CWE-312 SASL client password logged verbatim at tTd(95,5) debug level in getauth(); MOADs 0001/0002/0003/0005 CLEAN - [x] PgBouncer (C, PostgreSQL connection pooler) — pgbouncer-0001 MOAD-0004 CWE-312 SCRAM verifier logged at slog_debug; pgbouncer-0002 MOAD-0001 CWE-407 find_database() O(D) linear scan per connection 100x at D=100; MOADs 0002/0003/0005 CLEAN (single-threaded libevent) - [x] Suricata (C, IDS/IPS) — suricata-0001 (CWE-407 threshold SID lookup O(T×S), CWE-312 auth header logging); suricata-0002 (CWE-407 EveHttpLogJSONHeaders O(H×F=53) per tx, 53x); MOADs 0003/0005 CLEAN diff --git a/defects/squid-0001/TICKET.md b/defects/squid-0001/TICKET.md new file mode 100644 index 000000000..3678f33ed --- /dev/null +++ b/defects/squid-0001/TICKET.md @@ -0,0 +1,43 @@ +# squid-0001 — CWE-407 O(S*D) NotePairs::appendNewOnly hasPair() linear scan + +**Target:** Squid (squid-cache/squid, depth=1, 2026-03-31) +**File:** `src/Notes.cc` +**Function:** `NotePairs::appendNewOnly(const NotePairs *src)` +**UNDF:** UNDF-2026-000000870 +**Severity:** MEDIUM +**MOAD:** 0001 +**Benchmark:** 374.8x op-count reduction at S=D=500 + +## Defect + +`NotePairs::appendNewOnly()` merges annotations from one `NotePairs` object into +another, skipping duplicates. For each source entry it calls `hasPair()`, which +linearly scans all existing destination entries: + +```c++ +void +NotePairs::appendNewOnly(const NotePairs *src) +{ + for (const auto &e: src->entries) { + if (!hasPair(e->name(), e->value())) // O(D) scan per source entry + entries.push_back(...); + } +} +``` + +Total complexity: O(S * D) where S = source entries, D = destination entries. +Called per HTTP request in `ClientHttpRequest::initRequest()` to merge connection +annotations into request annotations. At S=D=500 the defect performs 374,750 ops +vs 1,000 for our fix. + +## Fix + +Build a `std::set>` of existing (name, value) pairs before +the loop, reducing `hasPair()` from O(D) to O(log D). Total: O((S+D) log D) +instead of O(S * D). The set is also updated as new entries are added so that +duplicate-within-src entries are also caught correctly. + +## Test + +`test/SquidNotePairsTest.java` — pure Java simulation, no Squid install needed. +374.8x op-count ratio confirmed at S=D=500. diff --git a/defects/squid-0002/TICKET.md b/defects/squid-0002/TICKET.md new file mode 100644 index 000000000..91e072e80 --- /dev/null +++ b/defects/squid-0002/TICKET.md @@ -0,0 +1,41 @@ +# squid-0002 — CWE-407 O(H*C) removeConnectionHeaderEntries() strListIsMember inner loop + +**Target:** Squid (squid-cache/squid, depth=1, 2026-03-31) +**File:** `src/HttpHeader.cc` +**Function:** `HttpHeader::removeConnectionHeaderEntries()` +**UNDF:** UNDF-2026-000001162 +**Severity:** MEDIUM +**MOAD:** 0001 +**Benchmark:** 3.27x wallclock, 40x op-count at H=200 headers, C=50 Connection tokens + +## Defect + +`removeConnectionHeaderEntries()` strips hop-by-hop headers listed in our +`Connection:` header field. It calls `strListIsMember()` inside our `getEntry()` +loop: + +```c++ +// Old comment in code: "think: on-average-best nesting of the two loops" +while ((e = getEntry(&pos))) { + if (strListIsMember(&strConnection, e->name, ',')) // O(C) scan per header + delAt(pos, headers_deleted); +} +``` + +`strListIsMember()` tokenizes our `Connection` string on each call, doing O(C) +work per header entry. Total: O(H * C) per response hop. + +Called in `removeHopByHopEntries()` for every forwarded HTTP response. At H=200 +headers and C=50 Connection tokens: 10,000 string comparisons per response. + +## Fix + +Pre-build an `std::unordered_set` from Connection tokens once +before our header loop (O(C) inserts), then probe O(1) per header entry. Total: +O(C + H) instead of O(H * C). + +## Test + +`test/SquidConnHeaderTest.java` — pure Java simulation, no Squid install needed. +3.27x wallclock speedup confirmed at H=200, C=50. Correctness verified (same 3 +hop-by-hop headers removed in both paths). diff --git a/defects/squid-0003/TICKET.md b/defects/squid-0003/TICKET.md new file mode 100644 index 000000000..e05fce448 --- /dev/null +++ b/defects/squid-0003/TICKET.md @@ -0,0 +1,54 @@ +# squid-0003 — CWE-312 FTP and Basic auth credentials logged verbatim + +**Target:** Squid (squid-cache/squid, depth=1, 2026-03-31) +**Files:** `src/clients/FtpGateway.cc`, `src/auth/basic/Config.cc`, `src/auth/basic/UserRequest.cc` +**UNDF:** UNDF-2026-000001163 +**Severity:** HIGH (one site at DBG_IMPORTANT, logs without any debug tuning) +**MOAD:** 0004 (CWE-312) + +## Defect + +Three callsites log plaintext credentials to `cache.log`: + +### Site 1 — FtpGateway.cc loginParser() (debug level 9) +```c++ +debugs(9, 9, "IN : login=" << login << ", escaped=" << escaped + << ", user=" << user << ", password=" << password); +// ... later: +debugs(9, 9, "found password=" << pass << " ..."); +debugs(9, 9, "OUT: login=" << login << "..., password=" << password); +``` +Full FTP credentials logged at debug level 9 (enabled with `debug_options 9,9`). + +### Site 2 — auth/basic/Config.cc decodeCleartext() (DEBUG_IMPORTANT = level 1) +```c++ +debugs(29, 9, "'" << cleartext << "'"); // decoded user:password +debugs(29, DBG_IMPORTANT, "WARNING: Bad characters in authorization header '" + << httpAuthHeader << "'"); // raw base64 Authorization: header +``` +Our DBG_IMPORTANT site (section 29, level 1) fires whenever bad characters are +detected in a Basic auth header, without any `debug_options` tuning. The raw +`Authorization: Basic ` value is logged, which decodes trivially to +`user:password`. + +### Site 3 — auth/basic/UserRequest.cc startHelperLookup() (debug level 9) +```c++ +debugs(29, 9, "'" << basic_auth->username() << ":" << basic_auth->passwd << "'"); +``` +Logs `user:password` colon-separated at debug level 9. + +## Fix + +Replace all credential values with redacted markers: +- `[REDACTED]` for passwords +- `[user]` for usernames where only context is needed +- Log lengths/flags instead of values +- DBG_IMPORTANT site: suppress base64 Authorization header entirely + +## Test + +`test/SquidCredentialLogTest.java` — simulates all three log sites. +Confirms: defective path contains `s3cr3t!FTP`, `proxyuser:myP@ssw0rd`. +Fixed path contains only `[REDACTED]`, lengths, and safe metadata. +DBG_IMPORTANT path: verifies base64 header not present in output. +PASS.