From dc35f8ea4bb9df55d81cddb3695c86235b31b71f Mon Sep 17 00:00:00 2001 From: jevb Date: Tue, 31 Mar 2026 19:11:36 +0200 Subject: [PATCH] fix: client security hardening (19 fixes across Rust + TypeScript) Addresses findings from comprehensive security review of the Tauri client: Critical: - Scope fs:allow-write-file from ** to $APPDATA/**,$APPLOG/** - Validate server_url scheme (https://) in update_commands.rs High: - Change CRED_PERSIST_LOCAL_MACHINE to CRED_PERSIST_ENTERPRISE (per-user) - Remove password from IPC response (#[serde(skip)] on CredentialData) - Auto-login uses stored token instead of password - Gate open_devtools behind #[cfg(feature = "devtools")] at registration - Validate remote_host for CRLF/null in livekit_proxy - Guard icons.ts innerHTML with runtime check - Add file upload MIME type allowlist - Clear pendingTotpPartialToken after use Medium: - Add sandbox attribute to YouTube iframes - Remove image/svg+xml from SAFE_MIME_TYPES - Strip trailing punctuation from linkified URLs - Validate host format in api.ts setConfig - Cap error messages at 200 chars (anti-phishing) - Rate limit search requests (500ms interval) - Validate Tenor GIF URLs against trusted origins - Sanitize notification titles (control chars + length cap) - Validate ptt_set_key vk_code range (1-254) - Add host validation to store_cert_fingerprint Docs: - Add "Client Security Hardening" section to docs/security.md --- .../src-tauri/capabilities/default.json | 5 +- Client/tauri-client/src-tauri/src/commands.rs | 8 +++- .../tauri-client/src-tauri/src/credentials.rs | 11 +++-- Client/tauri-client/src-tauri/src/lib.rs | 1 + .../src-tauri/src/livekit_proxy.rs | 10 ++++ Client/tauri-client/src-tauri/src/ptt.rs | 7 ++- .../src-tauri/src/update_commands.rs | 19 ++++++++ .../tauri-client/src/components/FileUpload.ts | 22 ++++++++- .../src/components/SearchOverlay.ts | 7 +++ .../components/message-list/attachments.ts | 12 ++++- .../components/message-list/content-parser.ts | 13 +++-- .../src/components/message-list/media.ts | 1 + Client/tauri-client/src/lib/api.ts | 15 +++++- Client/tauri-client/src/lib/credentials.ts | 4 +- Client/tauri-client/src/lib/icons.ts | 9 ++-- Client/tauri-client/src/lib/notifications.ts | 14 ++++-- Client/tauri-client/src/lib/tenor.ts | 30 ++++++++++-- Client/tauri-client/src/main.ts | 48 ++++++++----------- Client/tauri-client/src/pages/ConnectPage.ts | 3 +- .../src/pages/connect-page/LoginForm.ts | 4 ++ .../src/pages/connect-page/ServerPanel.ts | 2 +- .../tests/unit/server-panel.test.ts | 1 - docs/security.md | 44 +++++++++++++++++ 23 files changed, 233 insertions(+), 57 deletions(-) diff --git a/Client/tauri-client/src-tauri/capabilities/default.json b/Client/tauri-client/src-tauri/capabilities/default.json index d0dbc171..867fcccc 100644 --- a/Client/tauri-client/src-tauri/capabilities/default.json +++ b/Client/tauri-client/src-tauri/capabilities/default.json @@ -72,7 +72,10 @@ "identifier": "fs:allow-write-file", "allow": [ { - "path": "**" + "path": "$APPDATA/**" + }, + { + "path": "$APPLOG/**" } ] }, diff --git a/Client/tauri-client/src-tauri/src/commands.rs b/Client/tauri-client/src-tauri/src/commands.rs index cc2e3a08..00515ae2 100644 --- a/Client/tauri-client/src-tauri/src/commands.rs +++ b/Client/tauri-client/src-tauri/src/commands.rs @@ -78,8 +78,12 @@ pub fn store_cert_fingerprint( // Normalize to lowercase for consistent comparison with ws_proxy fingerprints let fingerprint = fingerprint.to_lowercase(); - if host.is_empty() { - return Err("host must not be empty".into()); + if host.is_empty() || host.len() > 253 { + return Err("host must be 1-253 characters".into()); + } + // Validate host format: alphanumeric, dots, hyphens, colons (port), brackets (IPv6) + if !host.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | ':' | '[' | ']')) { + return Err("host contains invalid characters".into()); } if fingerprint.is_empty() { return Err("fingerprint must not be empty".into()); diff --git a/Client/tauri-client/src-tauri/src/credentials.rs b/Client/tauri-client/src-tauri/src/credentials.rs index 71f1e970..465f63ba 100644 --- a/Client/tauri-client/src-tauri/src/credentials.rs +++ b/Client/tauri-client/src-tauri/src/credentials.rs @@ -2,9 +2,12 @@ use serde::Serialize; use std::ptr; use windows::core::{PCWSTR, PWSTR}; use windows::Win32::Foundation::ERROR_NOT_FOUND; +// CRED_PERSIST_ENTERPRISE scopes credentials per-user (roams with domain +// profile). Previously CRED_PERSIST_LOCAL_MACHINE was used, which exposes +// credentials to all users on shared machines. use windows::Win32::Security::Credentials::{ CredDeleteW, CredFree, CredReadW, CredWriteW, CREDENTIALW, CRED_FLAGS, - CRED_PERSIST_LOCAL_MACHINE, CRED_TYPE_GENERIC, + CRED_PERSIST_ENTERPRISE, CRED_TYPE_GENERIC, }; /// Data returned from `load_credential`. @@ -12,7 +15,9 @@ use windows::Win32::Security::Credentials::{ pub struct CredentialData { pub username: String, pub token: String, - #[serde(skip_serializing_if = "Option::is_none")] + // Password is stored in the credential blob for re-authentication but + // is never serialized back to the frontend over IPC to limit exposure. + #[serde(skip)] pub password: Option, } @@ -82,7 +87,7 @@ pub fn save_credential(host: String, username: String, token: String, password: LastWritten: Default::default(), CredentialBlobSize: blob.len() as u32, CredentialBlob: blob.as_ptr() as *mut u8, - Persist: CRED_PERSIST_LOCAL_MACHINE, + Persist: CRED_PERSIST_ENTERPRISE, AttributeCount: 0, Attributes: ptr::null_mut(), TargetAlias: PWSTR::null(), diff --git a/Client/tauri-client/src-tauri/src/lib.rs b/Client/tauri-client/src-tauri/src/lib.rs index be4e43d4..ad975e52 100644 --- a/Client/tauri-client/src-tauri/src/lib.rs +++ b/Client/tauri-client/src-tauri/src/lib.rs @@ -42,6 +42,7 @@ pub fn run() { ptt::ptt_listen_for_key, livekit_proxy::start_livekit_proxy, livekit_proxy::stop_livekit_proxy, + #[cfg(feature = "devtools")] commands::open_devtools, ]) .setup(|app| { diff --git a/Client/tauri-client/src-tauri/src/livekit_proxy.rs b/Client/tauri-client/src-tauri/src/livekit_proxy.rs index 060c576e..abbaed19 100644 --- a/Client/tauri-client/src-tauri/src/livekit_proxy.rs +++ b/Client/tauri-client/src-tauri/src/livekit_proxy.rs @@ -189,6 +189,16 @@ pub async fn start_livekit_proxy( state: tauri::State<'_, LiveKitProxyState>, remote_host: String, ) -> Result { + // Reject remote_host values containing CRLF or null bytes to prevent + // HTTP header injection in the proxy's header rewriting logic. + if remote_host.contains('\r') || remote_host.contains('\n') || remote_host.contains('\0') { + return Err("remote_host contains invalid characters".into()); + } + // Basic hostname format: alphanumeric, dots, hyphens, colons (port), brackets (IPv6) + if !remote_host.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | ':' | '[' | ']')) { + return Err("remote_host contains unexpected characters".into()); + } + let mut inner = state.inner.lock().await; info!("[livekit_proxy] start requested for {}", remote_host); diff --git a/Client/tauri-client/src-tauri/src/ptt.rs b/Client/tauri-client/src-tauri/src/ptt.rs index 4e96810b..53cc5f62 100644 --- a/Client/tauri-client/src-tauri/src/ptt.rs +++ b/Client/tauri-client/src-tauri/src/ptt.rs @@ -57,9 +57,14 @@ pub fn ptt_stop() { } /// Set the PTT virtual key code. Pass 0 to disable. +/// Valid range: 0 (disabled) or 1–254 (Windows virtual key codes). #[tauri::command] -pub fn ptt_set_key(vk_code: i32) { +pub fn ptt_set_key(vk_code: i32) -> Result<(), String> { + if vk_code != 0 && !(1..=254).contains(&vk_code) { + return Err(format!("invalid virtual key code: {vk_code} (must be 0 or 1-254)")); + } PTT_VKEY.store(vk_code, Ordering::SeqCst); + Ok(()) } /// Get the current PTT virtual key code. diff --git a/Client/tauri-client/src-tauri/src/update_commands.rs b/Client/tauri-client/src-tauri/src/update_commands.rs index 6e9057ab..3956c896 100644 --- a/Client/tauri-client/src-tauri/src/update_commands.rs +++ b/Client/tauri-client/src-tauri/src/update_commands.rs @@ -9,6 +9,21 @@ pub struct UpdateCheckResult { pub body: Option, } +/// Validate that a server URL is safe for the updater to connect to. +fn validate_server_url(server_url: &str) -> Result<(), String> { + let trimmed = server_url.trim_end_matches('/'); + if !trimmed.starts_with("https://") { + return Err("server_url must use https:// scheme".into()); + } + // Reject URLs with userinfo (e.g. "https://evil@host") + if let Ok(parsed) = url::Url::parse(trimmed) { + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err("server_url must not contain userinfo".into()); + } + } + Ok(()) +} + /// Check for a client update using the given server URL to build the endpoint /// dynamically. This is required because OwnCord is self-hosted and the /// server address varies per user. @@ -17,6 +32,8 @@ pub async fn check_client_update( app: AppHandle, server_url: String, ) -> Result { + validate_server_url(&server_url)?; + let current_version = app .config() .version @@ -71,6 +88,8 @@ pub async fn download_and_install_update( app: AppHandle, server_url: String, ) -> Result<(), String> { + validate_server_url(&server_url)?; + let current_version = app .config() .version diff --git a/Client/tauri-client/src/components/FileUpload.ts b/Client/tauri-client/src/components/FileUpload.ts index 5fd23445..6c550ff7 100644 --- a/Client/tauri-client/src/components/FileUpload.ts +++ b/Client/tauri-client/src/components/FileUpload.ts @@ -5,9 +5,19 @@ import { createElement, setText, appendChildren } from "@lib/dom"; import { createIcon } from "@lib/icons"; import type { MountableComponent } from "@lib/safe-render"; +/** Default allowed MIME types for file uploads. */ +const DEFAULT_ALLOWED_TYPES = [ + "image/jpeg", "image/png", "image/gif", "image/webp", "image/avif", + "video/mp4", "video/webm", + "audio/mpeg", "audio/ogg", "audio/wav", + "application/pdf", + "text/plain", +]; + export interface FileUploadOptions { readonly onUpload: (file: File) => Promise; readonly maxSizeMb?: number; + readonly allowedMimeTypes?: readonly string[]; } const DEFAULT_MAX_SIZE_MB = 10; @@ -69,6 +79,11 @@ export function createFileUpload(options: FileUploadOptions): FileUploadComponen async function handleFile(file: File): Promise { errorDiv.classList.add("file-upload__error--hidden"); + const allowed = options.allowedMimeTypes ?? DEFAULT_ALLOWED_TYPES; + if (file.type && !allowed.includes(file.type)) { + showError(`File type "${file.type}" is not allowed.`); + return; + } if (file.size > maxBytes) { showError(`File too large (${formatSize(file.size)}). Max ${options.maxSizeMb ?? DEFAULT_MAX_SIZE_MB} MB.`); return; @@ -93,7 +108,12 @@ export function createFileUpload(options: FileUploadOptions): FileUploadComponen dropzone = createElement("div", { class: "file-upload__dropzone file-upload__dropzone--hidden" }); appendChildren(dropzone, createElement("span", { class: "file-upload__droptext" }, "Drop files here")); - fileInput = createElement("input", { class: "file-upload__input", type: "file" }); + const allowed = options.allowedMimeTypes ?? DEFAULT_ALLOWED_TYPES; + fileInput = createElement("input", { + class: "file-upload__input", + type: "file", + accept: allowed.join(","), + }); fileInput.style.display = "none"; preview = createElement("div", { class: "file-upload__preview file-upload__preview--hidden" }); diff --git a/Client/tauri-client/src/components/SearchOverlay.ts b/Client/tauri-client/src/components/SearchOverlay.ts index 9a5d0657..9f4181bb 100644 --- a/Client/tauri-client/src/components/SearchOverlay.ts +++ b/Client/tauri-client/src/components/SearchOverlay.ts @@ -25,6 +25,8 @@ export interface SearchOverlayOptions { const DEBOUNCE_MS = 300; const MIN_QUERY_LEN = 2; +/** Minimum interval between actual search API calls (rate limiting). */ +const MIN_SEARCH_INTERVAL_MS = 500; // --------------------------------------------------------------------------- // Factory @@ -42,6 +44,7 @@ export function createSearchOverlay(options: SearchOverlayOptions): MountableCom let results: readonly SearchResultItem[] = []; let debounceTimer: number | null = null; let searchAbort: AbortController | null = null; + let lastSearchTime = 0; function formatTimestamp(ts: string): string { try { @@ -99,6 +102,10 @@ export function createSearchOverlay(options: SearchOverlayOptions): MountableCom } function doSearch(): void { + const now = Date.now(); + if (now - lastSearchTime < MIN_SEARCH_INTERVAL_MS) return; + lastSearchTime = now; + const query = input.value.trim(); if (query.length < MIN_QUERY_LEN) { results = []; diff --git a/Client/tauri-client/src/components/message-list/attachments.ts b/Client/tauri-client/src/components/message-list/attachments.ts index ef2eb79a..ea273c87 100644 --- a/Client/tauri-client/src/components/message-list/attachments.ts +++ b/Client/tauri-client/src/components/message-list/attachments.ts @@ -77,8 +77,11 @@ export function clearAttachmentCaches(): void { } /** Safe MIME types allowed in data: URIs — blocks script injection via crafted Content-Type. */ +// Note: image/svg+xml is intentionally excluded — SVGs can execute JS if +// loaded in , , or