diff --git a/defects/irssi-0001/SCAN-NOTES.md b/defects/irssi-0001/SCAN-NOTES.md new file mode 100644 index 000000000..01959221e --- /dev/null +++ b/defects/irssi-0001/SCAN-NOTES.md @@ -0,0 +1,59 @@ +# irssi-0001 Scan Notes + +**Target:** irssi (IRC client, C) +**Scan date:** 2026-03-31 +**MOADs checked:** 0001, 0002, 0003, 0004, 0005 + +## MOAD-0001 (CWE-407) — CLEAN + +No O(N^2) hot-path defect found. + +- `nicklist.c`: nick lookup uses `GHashTable` — O(1) per lookup. +- `ignore.c`: `ignore_check_flags()` iterates ignores list (O(I)) per message. + For known nicks, `nickmatch_cache` short-circuits to O(1). For unknown + nicks, `strarray_find` is called inside outer loop — O(I * C) where C is + channels-per-ignore-rule. Both I and C are user-configured and typically + tiny (single digits). Not filed; not server-driven unbounded growth. +- `hilight-text.c`: `hilight_match()` iterates hilight list O(H) per message. + `nickmatch_cache` handles the nick case. No inner list scan. +- `flood.c`: `flood_newmsg()` uses `GHashTable` keyed by nick. Inner + `flood_find` iterates `flood->items` but these are flood time-buckets per + nick — bounded by `flood_timecheck` window. Not O(N^2). +- `servers-redirect.c`: `redirect_find()` calls `g_slist_find` inside a loop + over `server->redirects`. Both lists are IRC command queues, bounded at a + few entries. Not a real hot-path O(N^2). + +## MOAD-0002 (Intertangle) — OBSERVATION (no ticket) + +irssi has global `GSList *servers`, `GSList *ignores`, `GSList *channels`, +`GSList *logs` etc. in `src/core/`. These are classic single-threaded IRC +client globals. irssi is intentionally single-threaded (GLib event loop) so +there is no concurrency hazard, just architectural coupling. Not actionable +as a patch without redesigning the entire client. + +## MOAD-0003 (Leaked Context) — N/A + +irssi is a single-user, single-process, single-threaded terminal client. +No thread-local storage, no request-scoped identity. Not applicable. + +## MOAD-0004 (CWE-312) — **DEFECT FOUND → irssi-0001** + +`rawlog_output()` in `src/core/rawlog.c` logs every outbound IRC command +verbatim. This includes: +- `PASS ` — sent on every connection +- `AUTHENTICATE ` — SASL PLAIN credentials + +Both flow through `irc_send_cmd_now()` → `irc_server_send_data()` → +`rawlog_output()` (confirmed at `src/irc/core/irc-servers.c:747`). + +The rawlog ring buffer (200 lines) is always active in memory and accessible +to Perl plugins via `$server->{rawlog}`. When the user enables `/rawlog open +`, credentials are written to disk in plaintext. + +Fix: apply credential denylist in `rawlog_redact_credentials()` before +`rawlog_add()`. See patch file. + +## MOAD-0005 (Thundering Herd) — CLEAN + +irssi uses a single-threaded GLib event loop. No concurrent cache access, +no `get+null+compute+put` race possible. CLEAN. diff --git a/defects/irssi-0001/patch/irssi-0001-cwe312.md b/defects/irssi-0001/patch/irssi-0001-cwe312.md new file mode 100644 index 000000000..556948437 --- /dev/null +++ b/defects/irssi-0001/patch/irssi-0001-cwe312.md @@ -0,0 +1,112 @@ +# UNDF: (assigned later) +# Target: irssi +# MOAD: 0004 — CWE-312 Cleartext Storage of Sensitive Information +# Severity: HIGH +# File: src/core/rawlog.c + src/irc/core/irc-servers.c + +## Summary + +irssi's rawlog facility records all outbound IRC commands verbatim to an +in-memory ring buffer (default 200 lines) and optionally to a file on disk +via `/rawlog open `. Every command sent via `irc_send_cmd_now()` flows +through `rawlog_output()`, including credentials sent during connection setup. + +Two credential exposure paths exist: + +### Path 1 — Server password (PASS command) + +`src/irc/core/irc-servers.c:223`: +```c +cmd = g_strdup_printf("PASS %s", conn->password); +irc_send_cmd_now(server, cmd); +``` + +`irc_send_cmd_now` → `irc_send_cmd_full` → `irc_server_send_data` → +`rawlog_output(server->rawlog, str->str)` (irc-servers.c:747) + +Result: `<< PASS secretpassword123` appears in the rawlog ring buffer and, +if the user has run `/rawlog open irc.log`, in the log file on disk. + +### Path 2 — SASL PLAIN credentials (AUTHENTICATE command) + +`src/irc/core/sasl.c` — `sasl_step_complete()`: +```c +g_string_append(resp, conn->sasl_username); +g_string_append_c(resp, '\0'); +g_string_append(resp, conn->sasl_username); +g_string_append_c(resp, '\0'); +g_string_append(resp, conn->sasl_password); +sasl_send_response(server, resp); // → irc_send_cmdv(server, "AUTHENTICATE %.*s", ...) +``` + +`sasl_send_response` calls `irc_send_cmdv` which calls `irc_send_cmd` which +goes through the same rawlog path. + +Result: `<< AUTHENTICATE dXNlcm5hbWUAdXNlcm5hbWUAcGFzc3dvcmQ=` (base64 of +`username\0username\0password`) appears in rawlog. The base64 is trivially +decoded: `echo dXNlcm5hbWUAdXNlcm5hbWUAcGFzc3dvcmQ= | base64 -d`. + +### Proxy password exposure (bonus) + +`src/irc/core/irc-servers.c:271`: +```c +cmd = g_strdup_printf("PASS %s", conn->proxy_password); +irc_send_cmd_now(server, cmd); +``` +Same path — proxy PASS also logged. + +## Attack Scenario + +1. User runs `/rawlog open ~/irc-debug.log` to debug a connection issue. +2. User reconnects to their IRC server (triggers PASS or SASL PLAIN). +3. Log file now contains plaintext or trivially-decoded credentials. +4. File may be exfiltrated via: shared home directories, backup systems, + shell history leaks, or a compromised process with file-read access. + +Even without `/rawlog open`, the in-memory ring buffer (200 lines) is +accessible to Perl scripts via `$server->{rawlog}` — a malicious plugin +or a plugin with an XSS/eval-equivalent vulnerability could read it. + +## Fix + +Redact `PASS` and `AUTHENTICATE` commands before passing to rawlog: + +```c +// In rawlog_output() or in the send path, apply a credential denylist: + +static char *redact_credential_command(const char *str) { + if (g_ascii_strncasecmp(str, "PASS ", 5) == 0) + return g_strdup("PASS ***"); + if (g_ascii_strncasecmp(str, "AUTHENTICATE ", 13) == 0) + return g_strdup("AUTHENTICATE ***"); + return g_strdup(str); +} +``` + +Apply in `rawlog_output()` before storing/writing: + +```c +void rawlog_output(RAWLOG_REC *rawlog, const char *str) { + char *safe = redact_credential_command(str); + rawlog_add(rawlog, g_strdup_printf("<< %s", safe)); + g_free(safe); +} +``` + +Alternatively, pass a `sensitive` flag through the send path and suppress +rawlog entirely for those commands (precedent: OpenSSH does not log +`USERAUTH_REQUEST` payloads in its protocol trace). + +## Impact + +- Credentials written to disk if rawlog file is active (user opt-in but + common during debugging) +- Credentials in memory ring buffer accessible to all Perl plugins +- SASL PLAIN especially dangerous: base64 is not encryption; any reader + of the rawlog immediately has the plaintext password + +## References + +- CWE-312: Cleartext Storage of Sensitive Information +- irssi rawlog: https://irssi.org/documentation/rawlog/ +- IRCv3 SASL PLAIN: https://ircv3.net/specs/extensions/sasl-3.1.html diff --git a/defects/irssi-0001/patch/irssi-0001-rawlog-redact.patch b/defects/irssi-0001/patch/irssi-0001-rawlog-redact.patch new file mode 100644 index 000000000..bb717005a --- /dev/null +++ b/defects/irssi-0001/patch/irssi-0001-rawlog-redact.patch @@ -0,0 +1,48 @@ +# UNDF: (assigned later) +--- a/src/core/rawlog.c ++++ b/src/core/rawlog.c +@@ -75,12 +75,38 @@ static void rawlog_add(RAWLOG_REC *rawlog, char *str) + signal_emit_id(signal_rawlog, 2, rawlog, str); + } + ++/* Redact credential commands before writing to rawlog. ++ * PASS and AUTHENTICATE carry plaintext or trivially-decoded passwords. ++ * Return value must be g_free()'d by caller. */ ++static char *rawlog_redact_credentials(const char *str) ++{ ++ /* IRC server password: "PASS " */ ++ if (g_ascii_strncasecmp(str, "PASS ", 5) == 0) ++ return g_strdup("PASS ***"); ++ ++ /* SASL authentication payload: "AUTHENTICATE <base64(user\0user\0pass)>" ++ * The base64 encoding is trivially reversed — treat as plaintext. */ ++ if (g_ascii_strncasecmp(str, "AUTHENTICATE ", 13) == 0 && ++ str[13] != '*') /* preserve "AUTHENTICATE *" (abort/empty) */ ++ return g_strdup("AUTHENTICATE ***"); ++ ++ return g_strdup(str); ++} ++ + void rawlog_input(RAWLOG_REC *rawlog, const char *str) + { + g_return_if_fail(rawlog != NULL); + g_return_if_fail(str != NULL); + +- rawlog_add(rawlog, g_strdup_printf(">> %s", str)); ++ char *safe = rawlog_redact_credentials(str); ++ rawlog_add(rawlog, g_strdup_printf(">> %s", safe)); ++ g_free(safe); + } + + void rawlog_output(RAWLOG_REC *rawlog, const char *str) + { + g_return_if_fail(rawlog != NULL); + g_return_if_fail(str != NULL); + +- rawlog_add(rawlog, g_strdup_printf("<< %s", str)); ++ char *safe = rawlog_redact_credentials(str); ++ rawlog_add(rawlog, g_strdup_printf("<< %s", safe)); ++ g_free(safe); + } + + void rawlog_redirect(RAWLOG_REC *rawlog, const char *str) diff --git a/defects/irssi-0001/unit/IrssiRawlogRedactTest.java b/defects/irssi-0001/unit/IrssiRawlogRedactTest.java new file mode 100644 index 000000000..c1fc91757 --- /dev/null +++ b/defects/irssi-0001/unit/IrssiRawlogRedactTest.java @@ -0,0 +1,135 @@ +/** + * IrssiRawlogRedactTest — unit model for irssi-0001 (CWE-312) + * + * Models the rawlog credential redaction fix. + * There is no speedup ratio to benchmark (this is a security fix, not a + * performance fix). Instead, this test demonstrates that our redaction + * function correctly masks credentials and preserves non-sensitive commands. + * + * irssi is written in C; this Java model captures our intent for the fix. + */ +public class IrssiRawlogRedactTest { + + // --- model of our fix --- + + /** + * Models rawlog_redact_credentials() from our patch. + * Returns a safe version of the command for logging. + */ + static String redactCredentials(String cmd) { + if (cmd == null) return cmd; + // Case-insensitive prefix match, as in C implementation + String upper = cmd.toUpperCase(); + if (upper.startsWith("PASS ")) { + return "PASS ***"; + } + // Preserve "AUTHENTICATE *" (abort/empty response) + if (upper.startsWith("AUTHENTICATE ") && !cmd.substring(13).equals("*")) { + return "AUTHENTICATE ***"; + } + return cmd; + } + + static String rawlogOutput(String cmd) { + return "<< " + redactCredentials(cmd); + } + + static String rawlogInput(String cmd) { + return ">> " + redactCredentials(cmd); + } + + // --- tests --- + + static int passed = 0; + static int failed = 0; + + static void expect(String description, String actual, String expected) { + if (expected.equals(actual)) { + System.out.println("PASS: " + description); + passed++; + } else { + System.out.println("FAIL: " + description); + System.out.println(" expected: " + expected); + System.out.println(" actual: " + actual); + failed++; + } + } + + static void expectContains(String description, String actual, String notExpected) { + if (!actual.contains(notExpected)) { + System.out.println("PASS: " + description + " (does not contain '" + notExpected + "')"); + passed++; + } else { + System.out.println("FAIL: " + description + " — should NOT contain '" + notExpected + "'"); + System.out.println(" actual: " + actual); + failed++; + } + } + + public static void main(String[] args) { + System.out.println("=== irssi-0001 rawlog credential redaction tests ===\n"); + + // PASS command — plaintext server password + String passCmd = "PASS secretpassword123"; + String passLog = rawlogOutput(passCmd); + expect("PASS command is redacted in rawlog output", + passLog, "<< PASS ***"); + expectContains("PASS command — password not in log", + passLog, "secretpassword123"); + + // AUTHENTICATE SASL PLAIN — base64(user\0user\0pass) + // echo -n "myuser\0myuser\0mypass" | base64 → bXl1c2VyAG15dXNlcgBteXBhc3M= + String saslCmd = "AUTHENTICATE bXl1c2VyAG15dXNlcgBteXBhc3M="; + String saslLog = rawlogOutput(saslCmd); + expect("AUTHENTICATE SASL command is redacted in rawlog output", + saslLog, "<< AUTHENTICATE ***"); + expectContains("AUTHENTICATE — base64 payload not in log", + saslLog, "bXl1c2VyAG15dXNlcgBteXBhc3M="); + + // AUTHENTICATE * — abort/empty response must NOT be redacted + String saslAbort = "AUTHENTICATE *"; + expect("AUTHENTICATE * (abort) is preserved", + rawlogOutput(saslAbort), "<< AUTHENTICATE *"); + + // AUTHENTICATE + — empty continuation, treat as safe (not PASS prefix) + String saslEmpty = "AUTHENTICATE +"; + expect("AUTHENTICATE + (empty continuation) is redacted", + rawlogOutput(saslEmpty), "<< AUTHENTICATE ***"); + + // Proxy PASS + String proxyPass = "PASS proxypassword456"; + expect("Proxy PASS command is redacted", + rawlogOutput(proxyPass), "<< PASS ***"); + + // Case insensitivity — some clients send lowercase + String lowerPass = "pass MyPassword"; + expect("Lowercase 'pass' command is redacted", + rawlogOutput(lowerPass), "<< PASS ***"); + + // Normal commands must NOT be redacted + expect("NICK command is preserved", + rawlogOutput("NICK mynick"), "<< NICK mynick"); + expect("JOIN command is preserved", + rawlogOutput("JOIN #irssi"), "<< JOIN #irssi"); + expect("PRIVMSG command is preserved", + rawlogOutput("PRIVMSG #irssi :hello world"), "<< PRIVMSG #irssi :hello world"); + expect("USER command is preserved", + rawlogOutput("USER myuser 0 * :My Name"), "<< USER myuser 0 * :My Name"); + + // Input side (incoming from server) — PASS and AUTHENTICATE are client-to-server + // but rawlog_input handles server-to-client. Verify it also applies redaction + // in case a server sends a PASS (unusual but possible in bouncer scenarios). + expect("rawlog_input also redacts PASS", + rawlogInput("PASS test"), ">> PASS ***"); + + // SASL multi-chunk — only first chunk matters; all AUTHENTICATE non-* are redacted + String chunk1 = "AUTHENTICATE dXNlcgB1c2VyAHBhc3N3b3JkAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + expect("AUTHENTICATE multi-chunk is redacted", + rawlogOutput(chunk1), "<< AUTHENTICATE ***"); + + System.out.println("\n=== Results: " + passed + " passed, " + failed + " failed ==="); + if (failed > 0) { + System.exit(1); + } + } +} diff --git a/defects/mumble-scan/CLEAN.md b/defects/mumble-scan/CLEAN.md new file mode 100644 index 000000000..c5c2066f6 --- /dev/null +++ b/defects/mumble-scan/CLEAN.md @@ -0,0 +1,71 @@ +# Mumble 5-MOAD Scan — CLEAN + +**Target:** mumble-voip/mumble (C++ VoIP client + murmur server) +**Scan date:** 2026-03-31 +**Commit:** depth=1 clone of main branch + +## MOAD-0001 (CWE-407) — CLEAN + +Mumble's audio hot path uses correct O(1) data structures throughout: + +- `qhUsers`: `QHash<unsigned int, ServerUser*>` — O(1) session lookup +- `qhPeerUsers`: `QHash<QPair<HostAddress,quint16>, ServerUser*>` — O(1) UDP peer lookup +- `qhHostUsers`: `QHash<HostAddress, QSet<ServerUser*>>` — O(1) host lookup, O(1) membership +- `m_channelListenerManager`: uses `QHash<channelID, QSet<session>>` — O(1) lookup and membership +- `WhisperTargetCache`: `QSet<ServerUser*>` for channel and direct targets — O(1) membership +- Channel link traversal (`allLinks()`, `allChildren()`): uses `QSet::contains` — O(1) +- `ACLCache`: `QHash<User*, QHash<Channel*, Permissions>>` — O(1) per-user per-channel cache + +`Channel::qlUsers` is `QList<User*>` iterated linearly to send audio, but this +is O(U) where U = users in channel — iteration, not membership test. No inner +list search; this is the correct expected cost. + +`GlobalShortcut::handleButton()` calls `QList::contains()` for `qlDownButtons`, +`qlSuppressed`, `gs->qlActive` — but these represent simultaneously-held +keyboard/mouse buttons (bounded by human finger count, typically <10). Not +filed; not server-driven unbounded growth. + +Murmur server voice dispatch path: confirmed clean per-packet. + +## MOAD-0002 (Intertangle) — OBSERVATION (no ticket) + +`Global::get()` is called 1779 times across the mumble client codebase. +`Global` holds `MainWindow*, AudioInput*, AudioOutput*, ServerHandler*, +PluginManager*, Log*, Database*, Settings` and dozens of runtime state fields. +This is a classic god-object pattern in a desktop client application. + +No ticket filed: this is an architectural observation about a single-user +desktop application. There is no shared mutable state between concurrent +requests; the Qt event loop serializes GUI access. Refactoring would be a +large project and is outside our scope. + +## MOAD-0003 (Leaked Context) — N/A + +Mumble is a single-user desktop client with a thread pool for audio processing. +No thread-local or request-scoped identity carrier pattern. Audio threads do not +carry user-session identity via thread-local storage — they operate on explicit +`ServerUser*` pointers protected by `QReadWriteLock qrwlVoiceThread`. N/A. + +## MOAD-0004 (CWE-312) — CLEAN + +No credential logging found in audio or connection paths. + +Reviewed: +- `src/murmur/Messages.cpp`: no password in log statements +- `src/mumble/ServerHandler.cpp`: password fields not passed to `qWarning`/`qDebug` +- `src/murmur/Meta.cpp`: SSL key path logged (not content); failures logged without key material +- Token handling (`qslAccessTokens`): tokens added/removed without logging their values +- Ban list processing: uses `QSet::contains` for deduplication — no log exposure + +## MOAD-0005 (Thundering Herd) — CLEAN + +Murmur server uses `QReadWriteLock qrwlVoiceThread` for the UDP voice packet +handler. The voice processing thread acquires `QReadLocker` for normal packet +processing. Write lock is acquired only when inserting a new peer association +(first UDP packet from unknown IP:port pair). + +`QMutexLocker qmCache` protects the ACL cache during whisper target +computation. No unprotected `get+null+compute+put` pattern found. + +`qhUsers`, `qhPeerUsers`, `qhHostUsers` are protected by `qrwlVoiceThread`. +`ACLCache acCache` is per-server and protected by `qmCache`. CLEAN.