irssi+mumble: 5-MOAD scan; irssi-0001 CWE-312 rawlog exposes PASS+AUTHENTICATE; mumble CLEAN
This commit is contained in:
parent
bbc12a3ca6
commit
b6d4a8ff18
5 changed files with 425 additions and 0 deletions
59
defects/irssi-0001/SCAN-NOTES.md
Normal file
59
defects/irssi-0001/SCAN-NOTES.md
Normal file
|
|
@ -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 <plaintext_server_password>` — sent on every connection
|
||||
- `AUTHENTICATE <base64(user\0user\0pass)>` — 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
|
||||
<file>`, 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.
|
||||
112
defects/irssi-0001/patch/irssi-0001-cwe312.md
Normal file
112
defects/irssi-0001/patch/irssi-0001-cwe312.md
Normal file
|
|
@ -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 <file>`. 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
|
||||
48
defects/irssi-0001/patch/irssi-0001-rawlog-redact.patch
Normal file
48
defects/irssi-0001/patch/irssi-0001-rawlog-redact.patch
Normal file
|
|
@ -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 <plaintext>" */
|
||||
+ 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)
|
||||
135
defects/irssi-0001/unit/IrssiRawlogRedactTest.java
Normal file
135
defects/irssi-0001/unit/IrssiRawlogRedactTest.java
Normal file
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
71
defects/mumble-scan/CLEAN.md
Normal file
71
defects/mumble-scan/CLEAN.md
Normal file
|
|
@ -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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue