Security Audit -- discordo-plus

Audited: 2026-03-20 | Auditor: Claude (automated)

CRITICAL

No critical vulnerabilities were found.

HIGH

1. ✅ FIXED — Path Traversal via Malicious Attachment Filenames

internal/ui/chat/messages_list.go:1202 -- The downloadToCache function joins a Discord-supplied attachment.Filename directly into a file path using filepath.Join(dir, attachment.Filename). Discord attachment filenames are server-controlled metadata; a malicious or compromised Discord server, webhook, or bot could supply a filename containing path traversal sequences (e.g., ../../../.bashrc or absolute paths on Windows). While filepath.Join on Go will clean .. sequences on most platforms, it does NOT sanitize on all edge cases (e.g., a filename that is simply .. followed by valid components, or Windows-specific \ separators). The same vulnerable pattern appears at line 1346 in saveAttachmentImage where attachment.Filename is joined with the user-configured image_save_dir. On that path, an attacker-controlled filename could overwrite arbitrary files in the save directory tree.

Fix: Sanitize the filename before joining. Extract only the base component and strip any path separators:

safeName := filepath.Base(attachment.Filename)
if safeName == "." || safeName == ".." || safeName == string(filepath.Separator) {
    safeName = "attachment"
}
path := filepath.Join(dir, safeName)

Apply this to both downloadToCache (line 1202) and saveAttachmentImage (line 1346).

Implementation Risk: Low. filepath.Base is the standard Go idiom for this. The only edge case is empty filenames (which Base returns as .), handled by the fallback. Existing cached files with clean filenames will continue to work.

2. ✅ FIXED — Editor Config Value Enables Command Injection via TOML

internal/config/editor.go (previously editor_unix.go) -- The CreateEditorCommand function on Unix previously used sh -c shell invocation, enabling command injection via TOML config or $EDITOR. Fixed by replacing sh -c with strings.Fields() + direct exec.Command. The two identical build-tagged files (editor_unix.go, editor_default.go) were merged into a single editor.go with no build tags. A slice aliasing bug in append(parts[1:], path) was also fixed by using explicit make+copy+append.

Implementation Risk: Low. Users relying on shell features in editor strings (pipes, env expansion) will need to adjust. The editor field now uses direct argument splitting, not shell evaluation.

3. ✅ FIXED — Overly Permissive File Permissions (os.ModePerm / 0777)

internal/logger/logger.go:20,24, internal/consts/consts.go:25, internal/ui/chat/messages_list.go:1198,1341, internal/notifications/notifications.go:78 -- All os.MkdirAll and os.OpenFile calls use os.ModePerm (0777), which creates directories and files that are world-readable and world-writable. The log file at ~/.cache/discordo/logs.txt is created with 0777 permissions. The log file records HTTP request URLs (which include the Discord API base but could contain query parameters in debug mode), error messages, and operational state. The attachment cache directory and saved images are also created with world-accessible permissions.

On a multi-user Linux system, any local user can read the log file, cached attachments, saved images, and the guild state file. While the Discord token is not directly logged, ws.EnableRawEvents = true in debug mode could cause raw gateway events (which transit the token-authenticated WebSocket) to be processed, and error messages may leak partial request/response data.

Fix: Use restrictive permissions:

// Directories: 0700 (owner only)
os.MkdirAll(dir, 0700)

// Regular files: 0600 (owner only)
os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)

Apply consistently across all six call sites.

Implementation Risk: Low. No other users or processes should need access to these files. Existing files will retain their current permissions until recreated.

MEDIUM

4. ✅ FIXED — Unbounded HTTP Response Body in Attachment Downloads

internal/ui/chat/messages_list.go:1191-1211 -- The downloadToCache function fetches an attachment via http.Get(attachment.URL) and copies the entire response body to disk with io.Copy(file, resp.Body). There is no size limit. A malicious Discord message with an attachment URL pointing to an extremely large file (or a server that streams infinite data) could fill the disk. The same issue exists in getCachedProfileImage in internal/notifications/notifications.go:93-99.

Fix: Use io.LimitReader to cap the download size:

maxSize := int64(100 * 1024 * 1024) // 100 MB
if _, err := io.Copy(file, io.LimitReader(resp.Body, maxSize)); err != nil {

Also check resp.ContentLength before starting the download when available.

Implementation Risk: Low. Discord has its own attachment size limits (typically 25-100 MB depending on tier), so a 100 MB cap is generous. The only behavioral change is rejecting pathologically large responses.

5. ✅ FIXED — Attachment Downloads Do Not Validate TLS or Content-Type

internal/ui/chat/messages_list.go:1191 and internal/notifications/notifications.go:93 -- Both attachment and avatar downloads use net/http.Get() (the stdlib default client) rather than the project's custom http.NewTransport(). This means these HTTP calls bypass the project's transport configuration. While Go's default HTTP client does validate TLS certificates, the attachment URL is entirely server-controlled -- a malicious embed or attachment URL could point to http:// (non-TLS) endpoints, local network addresses (SSRF to http://169.254.169.254 for cloud metadata, http://localhost:* for local services), or file:// URIs.

Additionally, downloadToCache does not validate the HTTP response status code -- a 404 or 500 response body would be silently written to disk and then opened in the image viewer.

Fix: (a) Validate that the URL scheme is https:// before fetching. (b) Check resp.StatusCode is 200 before writing. (c) Consider using the project's custom transport for consistency.

parsed, err := url.Parse(attachment.URL)
if err != nil || parsed.Scheme != "https" {
    return "", fmt.Errorf("invalid attachment URL scheme: %s", attachment.URL)
}
// ... after http.Get:
if resp.StatusCode != http.StatusOK {
    return "", fmt.Errorf("unexpected status: %d", resp.StatusCode)
}

Implementation Risk: Low. Discord CDN URLs are always HTTPS. This only blocks non-HTTPS URLs, which are either malicious or broken.

6. ✅ FIXED — Image Viewer Config Allows Arbitrary Command Execution

internal/ui/chat/messages_list.go:1247-1257,1269 -- The viewerArgs function takes the image_viewer config value and uses it as the first argument to exec.Command. On the non-Unix code path (and even on Unix when not using sh -c), the viewer string is used directly as a binary name. If a user sets image_viewer to a value containing spaces (e.g., image_viewer = "my viewer"), only the first argument would be the binary name, but the heuristic at line 1248 (strings.Contains(viewer, "mpv ")) suggests the field is expected to be a simple binary name. The viewer value goes directly to exec.Command(args[0], args[1:]...) where args[0] is the unsanitized viewer string. Combined with a user-writable config file and the fact that the path argument comes from a downloaded (attacker-named) file, this chain could be exploited if the cached attachment path is crafted.

However, since exec.Command does NOT invoke a shell and the path is a separate argument, the practical injection risk is limited to the config value itself (which the user controls).

Fix: Validate that image_viewer resolves to an executable via exec.LookPath at config load time. Log a warning if it does not resolve:

if cfg.ImageViewer != "" {
    if _, err := exec.LookPath(cfg.ImageViewer); err != nil {
        slog.Warn("image_viewer not found in PATH", "viewer", cfg.ImageViewer, "err", err)
    }
}

Implementation Risk: Low. This is a validation-only change. Users with valid viewer paths are unaffected.

7. Debug Mode Enables Raw WebSocket Event Logging

cmd/root.go:31 -- When --log-level=debug is passed, ws.EnableRawEvents = true is set globally on the arikawa WebSocket library. This causes raw gateway event data to be processed and potentially logged. While the current onRaw handler in state.go:24-31 only logs the event code and type (not the raw data -- the event.Raw line is commented out), the EnableRawEvents flag is a global setting that could be picked up by other arikawa internals or future code changes. Raw gateway events may contain user tokens in initial payloads, private message content, and other sensitive data. The log file is written with world-readable permissions (finding #3).

Fix: Either (a) remove the ws.EnableRawEvents = true line entirely, since the handler already filters out raw data, or (b) ensure it is only enabled with an explicit --enable-raw-events flag that warns about the security implications. At minimum, fix the log file permissions (finding #3) to prevent other users from reading debug logs.

Implementation Risk: Low. Removing EnableRawEvents has no impact on normal operation; it only disables the raw event hook that is already commented out in the handler.

LOW

8. ✅ FIXED — DISCORDO_TOKEN Environment Variable Visible to All Local Processes

internal/ui/root/model.go:88 -- The DISCORDO_TOKEN environment variable is checked before the keyring at startup. On Linux, environment variables of any process are readable by the same UID via /proc/PID/environ. Users who set this in .bashrc or .zshrc expose their Discord token to all processes running as the same user. This is documented in TECHFILE.md (item 8) but has no in-app warning.

Fix: Log a warning at startup when DISCORDO_TOKEN is used:

if token := os.Getenv(tokenEnvVarKey); token != "" {
    slog.Warn("Using DISCORDO_TOKEN from environment. Environment variables are visible to all processes running as the same user. Consider using the keyring instead.")
    cmd = tokenCommand(token)
}

Implementation Risk: None. This is a logging-only change.

9. ✅ FIXED — Guild State File Written Without Atomic Replacement

internal/ui/chat/guildstate.go:42-48 -- The save() method writes the guild state to state.json using os.WriteFile, which truncates and writes in place. If the application crashes or is killed during the write, the file will be partially written or empty, causing data loss on next load. While guild state is non-sensitive (only guild IDs and expand/collapse booleans), the non-atomic write could also cause a brief window where the file is empty and readable by other processes.

Fix: Write to a temporary file in the same directory and rename atomically:

tmp := stateFilePath + ".tmp"
if err := os.WriteFile(tmp, data, 0644); err != nil { ... }
if err := os.Rename(tmp, stateFilePath); err != nil { ... }

Implementation Risk: Negligible. Rename is atomic on POSIX systems. On Windows, it may require the target to not exist; use os.Rename which handles this on Go 1.26+.

10. ✅ FIXED — Avatar Cache Path Uses Discord-Supplied Hash Without Sanitization

internal/notifications/notifications.go:82 -- The avatar image cache path is constructed as filepath.Join(path, avatarHash+".png") where avatarHash is a discord.Hash (string type) received from the Discord API. While Discord avatar hashes are typically hex strings, the type is an alias for string and could theoretically contain path separators if the API response is manipulated. The same filepath.Base sanitization from finding #1 should be applied.

Fix: avatarHash = filepath.Base(string(avatarHash)) before path construction.

Implementation Risk: Negligible.

INFORMATIONAL

11. Commented-Out Local Replace Directives in go.mod

go.mod:5-7 -- The file contains commented-out replace directives pointing to local sibling directories (../tview, ../arikawa). These are development conveniences but could confuse contributors or be accidentally uncommented, causing builds to use unvetted local code.

Fix: Document these in CLAUDE.md (already done) and consider removing them from the committed go.mod, using go.work instead for local development.

Implementation Risk: None.

12. No Certificate Pinning for Discord API Connections

The project relies on Go's default TLS verification for all Discord API and CDN connections. This is standard and acceptable for a desktop application, but provides no protection against a compromised CA issuing a fraudulent certificate for discord.com. Certificate pinning would add defense-in-depth but is not standard practice for TUI applications.

Fix: No action needed. This is informational only.

Implementation Risk: N/A.

13. QR Login WebSocket Uses Default Dialer Without Custom TLS Config

internal/ui/login/qr/events.go:40 -- The QR login flow connects to wss://remote-auth-gateway.discord.gg using websocket.DefaultDialer, which uses Go's default TLS settings. This is functionally correct but does not benefit from the project's custom HTTP transport (gzip/brotli decompression).

Fix: No action needed for security. The default TLS configuration in Go is secure.