From c25a1c9e3325d0cf87d3c4f2d2fda819f5d5150f Mon Sep 17 00:00:00 2001 From: echobt Date: Wed, 4 Feb 2026 15:51:53 +0000 Subject: [PATCH 1/2] fix(tui): prevent path traversal in MCP storage via server name sanitization The server_path() function was vulnerable to path traversal attacks where a malicious server name like '../../../etc/passwd' could escape the mcps directory and access arbitrary files. Changes: - Add sanitize_server_name() function that replaces dangerous characters - Add validate_server_name() for pre-validation of untrusted input - Only alphanumeric, hyphen, and underscore characters are allowed - Path separators and other special chars are replaced with underscores - Add comprehensive unit tests for path traversal prevention Security Impact: Prevents directory traversal attacks that could lead to unauthorized file access or manipulation outside the MCP storage directory. Fixes: issue #5403 --- src/cortex-tui/src/mcp_storage.rs | 76 ++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/src/cortex-tui/src/mcp_storage.rs b/src/cortex-tui/src/mcp_storage.rs index 19b247b..cc4f845 100644 --- a/src/cortex-tui/src/mcp_storage.rs +++ b/src/cortex-tui/src/mcp_storage.rs @@ -15,6 +15,37 @@ use anyhow::{Context, Result}; use cortex_common::AppDirs; use serde::{Deserialize, Serialize}; +// ============================================================ +// SECURITY HELPERS +// ============================================================ + +/// Sanitize a server name to prevent path traversal attacks. +/// +/// Only allows alphanumeric characters, hyphens, and underscores. +/// Any other characters (including path separators) are replaced with underscores. +fn sanitize_server_name(name: &str) -> String { + name.chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '_' { + c + } else { + '_' + } + }) + .collect() +} + +/// Validate a server name for safe filesystem use. +/// +/// Returns true if the name contains only safe characters. +#[allow(dead_code)] +pub fn validate_server_name(name: &str) -> bool { + !name.is_empty() + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') +} + /// MCP transport type #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "lowercase")] @@ -158,8 +189,11 @@ impl McpStorage { } /// Get the path to a server's config file + /// + /// The server name is sanitized to prevent path traversal attacks. fn server_path(&self, name: &str) -> PathBuf { - self.mcps_dir.join(format!("{}.json", name)) + let sanitized_name = sanitize_server_name(name); + self.mcps_dir.join(format!("{}.json", sanitized_name)) } /// Save an MCP server configuration @@ -372,4 +406,44 @@ mod tests { let result = storage.load_server("nonexistent").unwrap(); assert!(result.is_none()); } + + #[test] + fn test_sanitize_server_name() { + // Normal names stay the same + assert_eq!(sanitize_server_name("my-server"), "my-server"); + assert_eq!(sanitize_server_name("server_123"), "server_123"); + + // Path traversal attempts get sanitized + assert_eq!(sanitize_server_name("../../../etc"), "________etc"); + assert_eq!(sanitize_server_name("test/subdir"), "test_subdir"); + assert_eq!(sanitize_server_name("test\\windows"), "test_windows"); + } + + #[test] + fn test_validate_server_name() { + // Valid names + assert!(validate_server_name("my-server")); + assert!(validate_server_name("server_123")); + assert!(validate_server_name("ABC")); + + // Invalid names + assert!(!validate_server_name("../../../etc")); + assert!(!validate_server_name("test/subdir")); + assert!(!validate_server_name("")); + assert!(!validate_server_name("name with spaces")); + } + + #[test] + fn test_server_path_traversal() { + let (storage, tmp) = test_storage(); + let base_dir = tmp.path().to_path_buf(); + + // Attempt path traversal + let malicious_name = "../../../etc/passwd"; + let result_path = storage.server_path(malicious_name); + + // The result should still be under mcps_dir + assert!(result_path.starts_with(base_dir.join("mcps"))); + assert!(!result_path.to_string_lossy().contains("..")); + } } From e7a17ae3a7737b54f92e51920c07a134dcbca0b6 Mon Sep 17 00:00:00 2001 From: echobt Date: Wed, 4 Feb 2026 16:13:59 +0000 Subject: [PATCH 2/2] fix: apply rustfmt formatting --- src/cortex-tui/src/mcp_storage.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cortex-tui/src/mcp_storage.rs b/src/cortex-tui/src/mcp_storage.rs index cc4f845..19cb8d7 100644 --- a/src/cortex-tui/src/mcp_storage.rs +++ b/src/cortex-tui/src/mcp_storage.rs @@ -412,7 +412,7 @@ mod tests { // Normal names stay the same assert_eq!(sanitize_server_name("my-server"), "my-server"); assert_eq!(sanitize_server_name("server_123"), "server_123"); - + // Path traversal attempts get sanitized assert_eq!(sanitize_server_name("../../../etc"), "________etc"); assert_eq!(sanitize_server_name("test/subdir"), "test_subdir"); @@ -425,7 +425,7 @@ mod tests { assert!(validate_server_name("my-server")); assert!(validate_server_name("server_123")); assert!(validate_server_name("ABC")); - + // Invalid names assert!(!validate_server_name("../../../etc")); assert!(!validate_server_name("test/subdir")); @@ -437,11 +437,11 @@ mod tests { fn test_server_path_traversal() { let (storage, tmp) = test_storage(); let base_dir = tmp.path().to_path_buf(); - + // Attempt path traversal let malicious_name = "../../../etc/passwd"; let result_path = storage.server_path(malicious_name); - + // The result should still be under mcps_dir assert!(result_path.starts_with(base_dir.join("mcps"))); assert!(!result_path.to_string_lossy().contains(".."));