Skip to content

Commit dcf2d18

Browse files
committed
fix(tui): prevent path traversal in session storage via session_id sanitization
The session_dir() function was vulnerable to path traversal attacks where a malicious session_id like '../../../etc/passwd' could escape the sessions directory and access arbitrary files. Changes: - Add sanitize_session_id() function that replaces dangerous characters - Add validate_session_id() 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 sessions directory. Fixes: issue #5404
1 parent c398212 commit dcf2d18

File tree

1 file changed

+86
-1
lines changed

1 file changed

+86
-1
lines changed

src/cortex-tui/src/session/storage.rs

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,37 @@ const META_FILE: &str = "meta.json";
2121
/// History file name.
2222
const HISTORY_FILE: &str = "history.jsonl";
2323

24+
// ============================================================
25+
// SECURITY HELPERS
26+
// ============================================================
27+
28+
/// Sanitize a session ID to prevent path traversal attacks.
29+
///
30+
/// Only allows alphanumeric characters, hyphens, and underscores.
31+
/// Any other characters (including path separators) are replaced with underscores.
32+
fn sanitize_session_id(session_id: &str) -> String {
33+
session_id
34+
.chars()
35+
.map(|c| {
36+
if c.is_ascii_alphanumeric() || c == '-' || c == '_' {
37+
c
38+
} else {
39+
'_'
40+
}
41+
})
42+
.collect()
43+
}
44+
45+
/// Validate a session ID for safe filesystem use.
46+
///
47+
/// Returns true if the session_id contains only safe characters.
48+
pub fn validate_session_id(session_id: &str) -> bool {
49+
!session_id.is_empty()
50+
&& session_id
51+
.chars()
52+
.all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_')
53+
}
54+
2455
// ============================================================
2556
// SESSION STORAGE
2657
// ============================================================
@@ -49,8 +80,21 @@ impl SessionStorage {
4980
}
5081

5182
/// Gets the directory for a specific session.
83+
///
84+
/// # Security
85+
///
86+
/// The session_id is validated to prevent path traversal attacks.
87+
/// Only alphanumeric characters, hyphens, and underscores are allowed.
88+
///
89+
/// # Panics
90+
///
91+
/// This function does not panic but will return an invalid path if
92+
/// the session_id contains disallowed characters. Use `validate_session_id`
93+
/// before calling this function for untrusted input.
5294
pub fn session_dir(&self, session_id: &str) -> PathBuf {
53-
self.base_dir.join(session_id)
95+
// Sanitize session_id to prevent path traversal
96+
let sanitized_id = sanitize_session_id(session_id);
97+
self.base_dir.join(&sanitized_id)
5498
}
5599

56100
/// Gets the metadata file path for a session.
@@ -429,4 +473,45 @@ mod tests {
429473
let loaded = storage.load_meta(&session_id).unwrap();
430474
assert!(loaded.archived);
431475
}
476+
477+
#[test]
478+
fn test_validate_session_id() {
479+
// Valid IDs
480+
assert!(validate_session_id("abc-123"));
481+
assert!(validate_session_id("test_session"));
482+
assert!(validate_session_id("ABC123"));
483+
484+
// Invalid IDs - path traversal attempts
485+
assert!(!validate_session_id("../../../etc"));
486+
assert!(!validate_session_id(".."));
487+
assert!(!validate_session_id("test/../passwd"));
488+
assert!(!validate_session_id("test/subdir"));
489+
assert!(!validate_session_id(""));
490+
}
491+
492+
#[test]
493+
fn test_sanitize_session_id() {
494+
// Normal ID stays the same
495+
assert_eq!(sanitize_session_id("abc-123"), "abc-123");
496+
assert_eq!(sanitize_session_id("test_session"), "test_session");
497+
498+
// Path traversal gets sanitized
499+
assert_eq!(sanitize_session_id("../../../etc"), "________etc");
500+
assert_eq!(sanitize_session_id("test/subdir"), "test_subdir");
501+
assert_eq!(sanitize_session_id("test\x00evil"), "test_evil");
502+
}
503+
504+
#[test]
505+
fn test_session_dir_path_traversal() {
506+
let (storage, temp) = create_test_storage();
507+
let base_dir = temp.path().to_path_buf();
508+
509+
// Attempt path traversal - should be sanitized
510+
let malicious_id = "../../../etc/passwd";
511+
let result_path = storage.session_dir(malicious_id);
512+
513+
// The result should still be under base_dir, not escaping it
514+
assert!(result_path.starts_with(&base_dir));
515+
assert!(!result_path.to_string_lossy().contains(".."));
516+
}
432517
}

0 commit comments

Comments
 (0)