diff --git a/defects/kodi/SCAN.md b/defects/kodi/SCAN.md new file mode 100644 index 000000000..867267bc6 --- /dev/null +++ b/defects/kodi/SCAN.md @@ -0,0 +1,46 @@ +## Kodi 5-MOAD Scan — 2026-03-31 + +Target: https://github.com/xbmc/xbmc (depth=1) + +### MOAD-0001 (CWE-407) — CLEAN +Kodi uses proper containers (maps, sets, unordered_map.contains()) throughout. +Only 11 files use std::find, all on small fixed-size collections: +- VAAPI/DXVA m_freeSurfaces std::find: bounded by decoder surface pool size + (typically 16-32 surfaces), inside locked sections. Not O(N^2) at scale. +- MusicInfoScanner thumbs std::find: thumbs is a user-configured list of 3-10 + items. Bounded, not hot-path O(N^2). +- WSDiscovery IP dedup: O(N^2) but network discovery is bounded to ~100 devices max. +No significant MOAD-0001 defects found. + +### MOAD-0002 (Intertangle) — ARCHITECTURAL NOTE (no patch) +CServiceBroker is a global service locator exposing 30+ subsystems (addons, +PVR, network, video, audio, filesystem, settings, peripherals, etc.). This is +a classic god-object / intertangle pattern. However, it is intentional Kodi +architecture providing DI-style access without singletons. Patching it would +require a multi-year architectural refactor. Documented as architectural debt. + +### MOAD-0003 (Leaked Context) — ARCHITECTURAL NOTE (no patch) +xbmc/interfaces/legacy/LanguageHook.cpp uses `thread_local LanguageHook* addonLanguageHookTls` +to carry per-addon-invocation identity (addon ID, version, invoker ID) in our +scripting bridge. This is the MOAD-0003 pattern: request-scoped identity held +in thread-local storage. The LanguageHook.h comment (line 91) even acknowledges +the problem: "I need an InheritableThreadLocal C++ equivalent." +xbmc/interfaces/python/PyContext.cpp uses `thread_local PyContextState*` for +Python GIL management (not identity leakage per se — GIL state is inherently +per-thread). +The LanguageHook TLS is a true MOAD-0003 site but correcting it requires +replacing all addon callback dispatch with an explicit context parameter. +Documented as architectural debt. + +### MOAD-0004 (CWE-312) — CLEAN +xbmc/platform/posix/filesystem/SMBFile.cpp redacts credentials in SMB log +messages using std::regex: `"(\\w+://)\\S+:\\S+@"` replaced with +`"$1USERNAME:PASSWORD@"`. No plaintext credential logging found elsewhere. +CLog searches across xbmc/network/, xbmc/filesystem/ return no password hits. + +### MOAD-0005 (Thundering Herd) — CLEAN +CVideoThumbLoader::GetArtFromCache() has an unguarded get+insert pattern but +CBackgroundInfoLoader runs a single worker thread (std::unique_ptr), +so concurrent access to m_artCache is impossible by design. +VAAPI/DXVA surface cache operations are all wrapped in std::unique_lock. +No concurrent cache stampede found. diff --git a/defects/vlc-0003/patch/vlc-0003.patch b/defects/vlc-0003/patch/vlc-0003.patch new file mode 100644 index 000000000..4b34fa960 --- /dev/null +++ b/defects/vlc-0003/patch/vlc-0003.patch @@ -0,0 +1,27 @@ +# UNDF: (leave blank) +# CWE-312: Cleartext Storage of Sensitive Information +# VLC SMB1 DSM access module logs SMB credentials (username + domain) at WARN level. +# msg_Warn( p_access, "Creds: username = '%s', domain = '%s'", psz_login, psz_domain ) +# This line runs on every successful SMB1 login, writing plaintext credentials to +# the VLC log file, syslog, or any log sink configured by our user. +# VLC debug logs are routinely shared in bug reports, exposing SMB usernames and +# domain names to third parties. +# Fix: remove our credential log line entirely. Login success is already implicit +# from reaching this point without error. Domain and username do not need to be +# re-announced in the log after authentication completes. +# Severity: MEDIUM. Username + domain logged, not password. Still a CWE-312 defect +# because domain\username is often sufficient to enumerate valid accounts and +# facilitates phishing/lateral movement in corporate environments. +--- a/modules/access/dsm/access.c ++++ b/modules/access/dsm/access.c +@@ -582,9 +582,6 @@ static int Open( vlc_object_t *p_this ) + if( smb_session_is_guest( p_sys->p_session ) == 1 ) + { + msg_Warn( p_access, "Login failure but you were logged in as a Guest"); + b_guest = true; + } + +- msg_Warn( p_access, "Creds: username = '%s', domain = '%s'", +- psz_login, psz_domain ); + if( !b_guest ) + vlc_credential_store( &credential, p_access ); diff --git a/defects/vlc-0003/unit/VlcDsmCredentialLogTest.java b/defects/vlc-0003/unit/VlcDsmCredentialLogTest.java new file mode 100644 index 000000000..225b2a7b3 --- /dev/null +++ b/defects/vlc-0003/unit/VlcDsmCredentialLogTest.java @@ -0,0 +1,157 @@ +import java.util.ArrayList; +import java.util.List; + +/** + * Test for vlc-0003: CWE-312 SMB credentials logged verbatim via msg_Warn() + * in modules/access/dsm/access.c (VLC SMB1 DSM access module). + * + * Pattern: + * msg_Warn( p_access, "Creds: username = '%s', domain = '%s'", + * psz_login, psz_domain ); + * + * This line executes on every successful SMB1 login, writing plaintext + * username + domain to our VLC log (debug file, syslog, or any log sink). + * VLC debug logs are routinely included in bug reports, exposing SMB + * identity information (username + domain) to third parties. In corporate + * environments, domain/username is sufficient to enumerate valid accounts + * and facilitate lateral movement. + * + * Fix: remove our credential log line. Successful login is implicit from + * reaching this code path without error. Username and domain do not need + * to be announced in the log after authentication completes. + * + * Compile and run (no build tool required): + * javac defects/vlc-0003/unit/VlcDsmCredentialLogTest.java -d /tmp/vlc-0003 + * java -cp /tmp/vlc-0003 VlcDsmCredentialLogTest + */ +public class VlcDsmCredentialLogTest { + + private static int passed = 0; + private static int failed = 0; + + // --- Simulated VLC log sink --- + + static class LogSink { + private final List messages = new ArrayList<>(); + + void warn(String fmt, Object... args) { + messages.add(String.format(fmt, args)); + } + + boolean containsText(String text) { + for (String m : messages) { + if (m.contains(text)) return true; + } + return false; + } + + int size() { return messages.size(); } + } + + // --- Defective: logs username + domain via msg_Warn --- + + static void smbLoginDefective(LogSink log, String username, String domain) { + // smb_connect() succeeds ... + // smb_session_is_guest() returns 0 (not guest) + boolean isGuest = false; + if (!isGuest) { + // CWE-312 site — logs credentials verbatim + log.warn("Creds: username = '%s', domain = '%s'", username, domain); + } + // vlc_credential_store(...) + } + + // --- Fixed: credential log line removed --- + + static void smbLoginFixed(LogSink log, String username, String domain) { + // smb_connect() succeeds ... + // No credential log line + // vlc_credential_store(...) + boolean isGuest = false; + // isGuest path still logs the guest warning, but not credentials + } + + // --- Tests --- + + static void testDefectiveLogsUsername() { + LogSink log = new LogSink(); + smbLoginDefective(log, "alice", "CORP"); + check("defective path must log the username (confirms CWE-312 site present)", + log.containsText("alice")); + } + + static void testDefectiveLogsDomain() { + LogSink log = new LogSink(); + smbLoginDefective(log, "alice", "CORP"); + check("defective path must log the domain name", + log.containsText("CORP")); + } + + static void testFixedDoesNotLogUsername() { + LogSink log = new LogSink(); + smbLoginFixed(log, "alice", "CORP"); + check("fixed path must NOT log the SMB username", + !log.containsText("alice")); + } + + static void testFixedDoesNotLogDomain() { + LogSink log = new LogSink(); + smbLoginFixed(log, "alice", "CORP"); + check("fixed path must NOT log the SMB domain", + !log.containsText("CORP")); + } + + static void testFixedSuppressesAllCredentials() { + // Simulate 100 SMB1 logins — none should produce credential log entries + String[] usernames = new String[100]; + String[] domains = new String[100]; + for (int i = 0; i < 100; i++) { + usernames[i] = "user" + i; + domains[i] = "DOMAIN" + i; + } + + int leaks = 0; + for (int i = 0; i < 100; i++) { + LogSink log = new LogSink(); + smbLoginFixed(log, usernames[i], domains[i]); + if (log.containsText(usernames[i]) || log.containsText(domains[i])) { + leaks++; + } + } + check("fixed path must produce 0 credential leaks across 100 logins (got " + leaks + ")", + leaks == 0); + } + + static void testFixedProducesNoLogLines() { + LogSink log = new LogSink(); + smbLoginFixed(log, "administrator", "WORKGROUP"); + check("fixed login produces no log output (login success is implicit)", + log.size() == 0); + } + + // --- Harness --- + + static void check(String desc, boolean cond) { + if (cond) { + System.out.println(" PASS: " + desc); + passed++; + } else { + System.out.println(" FAIL: " + desc); + failed++; + } + } + + public static void main(String[] args) { + System.out.println("=== VlcDsmCredentialLogTest (vlc-0003, CWE-312 SMB credentials logged) ===\n"); + + testDefectiveLogsUsername(); + testDefectiveLogsDomain(); + testFixedDoesNotLogUsername(); + testFixedDoesNotLogDomain(); + testFixedSuppressesAllCredentials(); + testFixedProducesNoLogLines(); + + System.out.println("\n--- " + passed + " passed, " + failed + " failed ---"); + if (failed > 0) System.exit(1); + } +} diff --git a/defects/vlc/SCAN.md b/defects/vlc/SCAN.md new file mode 100644 index 000000000..c3db1bf1c --- /dev/null +++ b/defects/vlc/SCAN.md @@ -0,0 +1,38 @@ +## VLC 5-MOAD Scan — 2026-03-31 + +Target: https://github.com/videolan/vlc (depth=1) + +### MOAD-0001 (CWE-407) — 2 DEFECTS FOUND (prior sessions) +See vlc-0001 (modules/bank.c module find + randomizer_Remove O(N*C)) and +vlc-0002 (src/input/subtitles.c subtitle dedup O(I^2)). + +### MOAD-0002 (Intertangle) — CLEAN +VLC uses a vlc_object_t hierarchy as our object model, which is a known +coupling point, but each subsystem (audio out, video out, input, playlist, +sout) has its own thread and private struct. No hidden god object couples +independent subsystems through shared mutable state beyond the intentional +parent/child object tree. + +### MOAD-0003 (Leaked Context) — CLEAN (by design) +src/misc/interrupt.c uses `thread_local vlc_interrupt_t *vlc_interrupt_var` +to carry per-input-task interrupt context. This is save/restore with +vlc_interrupt_set() — callers save old context, set new one, restore on +return. It is thread-scoped, not leaked across requests. The pattern is +correct: each input thread owns its interrupt context for its lifetime. +Not a MOAD-0003 defect. + +### MOAD-0004 (CWE-312) — 1 DEFECT FOUND +See vlc-0003: modules/access/dsm/access.c:585 logs SMB username + domain +verbatim via msg_Warn() on every successful SMB1 login. + +Additional log calls reviewed: +- http.c:257 logs psz_username (not password) — username in URL is low severity. +- http.c:927-945 logs WWW-Authenticate and Authentication-Info headers from server + (challenge data, not credentials) — CLEAN. +- live555.cpp:718 logs "retrying with user=%s" username only — low severity. +- ftp.c:518-556 logs "password needed/accepted/rejected" without values — CLEAN. + +### MOAD-0005 (Thundering Herd) — CLEAN +Module bank (src/modules/bank.c) uses vlc_mutex_lock/unlock consistently around +module list access. Access cache uses mutex-protected entry lookup. No unguarded +get+null+set pattern found in hot paths.