discordo-plus/research/SECFILE.md

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. PREV-FIXED -- Debug Mode Enables Raw WebSocket Event Logging

cmd/root.go:31 -- When --log-level=debug is passed, ws.EnableRawEvents = true was set globally on the arikawa WebSocket library. This caused raw gateway event data to be processed and potentially logged. Raw gateway events may contain user tokens in initial payloads, private message content, and other sensitive data.

Fix: Removed ws.EnableRawEvents = true entirely. The raw event handler was already commented out, so this had no functional impact.

Implementation Risk: None.

---

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.

11. PREV-FIXED -- Cached Files Created with os.Create (Default 0666 Permissions)

internal/ui/chat/attachment_handler.go, internal/ui/chat/guildstate.go -- Several file creation paths used os.Create which applies the default umask-modified 0666 permissions, potentially leaving cached attachments and state files world-readable. Related to finding #3 but covering additional call sites added after the initial fix.

Fix: Replaced all os.Create calls with os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600) for consistent restrictive permissions across all file creation paths.

Implementation Risk: None.

---

12. PREV-FIXED -- Avatar Download Missing HTTPS Validation

internal/notifications/notifications.go -- The getCachedProfileImage function downloaded avatar images without validating the URL scheme, allowing potential HTTP (non-TLS) downloads of user avatars. Related to finding #5 but covering the avatar-specific code path.

Fix: Added scheme validation to require HTTPS before downloading avatar images, consistent with the attachment download fix in finding #5.

Implementation Risk: None. Discord CDN always uses HTTPS.

---

13. PREV-FIXED -- Emoji Favorites File Not Bounded

internal/ui/chat/emoji_picker.go -- The emoji favorites file (~/.cache/discordo/emoji_favorites.json) accepted arbitrary JSON with no size limit or entry count validation. A corrupted or maliciously crafted favorites file could contain an unbounded number of entries or excessively long strings.

Fix: Added a maximum of 10 favorites with string length validation. The loadFavorites function now validates entries before accepting them.

Implementation Risk: None.

---

14. PREV-FIXED -- Emoji Favorites Mutex Lock/Unlock Ordering

internal/ui/chat/emoji_picker.go -- The emojiFavorites.save() method acquired its own lock internally, but callers also held the lock, creating potential for deadlock or inconsistent state if the lock type changed. The RLock/Lock asymmetry between read and write paths was fragile.

Fix: Refactored so save() requires the caller to hold the lock (caller-must-hold-lock pattern). All call sites were updated to hold the write lock through the entire read-modify-save cycle.

Implementation Risk: None.

---

15. PREV-FIXED -- Compiled Binary Included in Repository

The compiled discordo-plus binary was previously present in the working directory. Binary artifacts should not be tracked in version control as they inflate repository size and may contain stale builds.

Fix: The binary is not git-tracked. .gitignore excludes the binary name. This was already resolved before the audit.

Implementation Risk: None.

---

INFORMATIONAL

16. 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.

17. 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.

18. 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.

Implementation Risk: N/A.

---

Summary

Severity	Total	Fixed
Critical	0	—
High	3	3
Medium	4 + 3 new	7 (all)
Low	3 + 2 new	5 (all)
Info	3	0

Top 3 Priorities

1. ✅ Path traversal via attachment filenames (#1) -- Fixed with filepath.Base sanitization.
2. ✅ File permission hardening (#3) -- Fixed with 0700/0600 permissions.
3. ✅ Editor command injection (#2) -- Fixed by replacing sh -c with direct exec.Command + strings.Fields.

All actionable findings are now resolved. Only informational items (#16-#18) remain, which require no action.

Positive Observations

• Token storage via OS keyring (zalando/go-keyring) is the correct default approach, avoiding plaintext token storage
• QR login uses proper RSA-2048 key generation with crypto/rand, SHA-256 OAEP decryption, and ephemeral key pairs
• WebSocket connection to Discord remote auth gateway uses WSS (TLS)
• Guild state persistence uses mutex correctly around all read/write operations
• The xdotool geometry detection degrades gracefully when the tool is absent
• Token is never logged -- slog calls related to token operations only log error messages, not the token value
• Build uses -trimpath to strip local filesystem paths from the binary
• The sendMessageData reference/mention handling correctly sets AllowedMentions to prevent unintended pings
• TOML config parsing uses the well-maintained BurntSushi/toml library with no known vulnerabilities
• Dependencies are overwhelmingly current (see TECHFILE.md) with no known CVEs in the active dependency set

---

Files Reviewed

Every file reviewed, listed for coverage verification.

#	File	Type
1	main.go	Entry point
2	cmd/root.go	CLI initialization
3	go.mod	Module manifest
4	.gitignore	Git config
5	.github/workflows/ci.yml	CI pipeline
6	internal/config/config.go	Config struct + loader
7	internal/config/config.toml	Embedded default config
8	internal/config/config_test.go	Config tests (grep)
9	internal/config/keybinds.go	Keybind definitions
10	internal/config/theme.go	Theme TOML types
11	internal/config/editor.go	Editor command (merged from editor_unix.go + editor_default.go)
13	internal/consts/consts.go	Constants + cache dir
14	internal/keyring/keyring.go	Token keyring access
15	internal/keyring/keyring_test.go	Keyring tests (presence verified)
16	internal/logger/logger.go	Log file setup
17	internal/http/transport.go	Custom HTTP transport
18	internal/http/client.go	API client factory
19	internal/http/headers.go	HTTP header construction
20	internal/http/props.go	Identify properties + super props
21	internal/clipboard/clipboard.go	Clipboard types
22	internal/clipboard/clipboard_default.go	Non-Linux clipboard
23	internal/clipboard/clipboard_wayland.go	Wayland clipboard (wl-copy)
24	internal/cache/cache.go	Completion cache
25	internal/markdown/renderer.go	Markdown AST renderer
26	internal/notifications/notifications.go	Notification dispatch
27	internal/notifications/desktop_toast.go	Linux/Windows toast
28	internal/notifications/desktop_toast_darwin.go	macOS notification
29	internal/ui/util.go	UI utilities
30	internal/ui/root/model.go	Root model + edit config
31	internal/ui/root/events.go	Root events (token, keyring)
32	internal/ui/root/keybinds.go	Root keybinds
33	internal/ui/root/suspend_unix.go	Unix SIGTSTP suspend
34	internal/ui/root/suspend_default.go	Non-Unix suspend no-op
35	internal/ui/chat/model.go	Chat model + state init
36	internal/ui/chat/messages_list.go	Message rendering + keybinds
36a	internal/ui/chat/attachment_handler.go	Attachment download/view/save
36b	internal/ui/chat/embed_renderer.go	Embed types + line wrapping
36c	internal/ui/chat/url_extractor.go	URL extraction from messages
37	internal/ui/chat/message_input.go	Message composition
38	internal/ui/chat/guilds_tree.go	Guild/channel tree
39	internal/ui/chat/guildstate.go	Guild state persistence
40	internal/ui/chat/state.go	Gateway state events
41	internal/ui/chat/events.go	Chat event types
42	internal/ui/chat/keybinds.go	Chat keybinds
43	internal/ui/chat/util.go	Chat utilities
44	internal/ui/chat/attachments_picker.go	Attachment picker
45	internal/ui/chat/channels_picker.go	Channel picker
46	internal/ui/chat/mentions_list.go	Mentions list
47	internal/ui/login/model.go	Login model
48	internal/ui/login/events.go	Login events
49	internal/ui/login/keybinds.go	Login keybinds
50	internal/ui/login/token/model.go	Token login form
51	internal/ui/login/token/events.go	Token events
52	internal/ui/login/qr/model.go	QR login model
53	internal/ui/login/qr/events.go	QR login events + crypto