diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..0d9f239 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2026-05-04 - Path Traversal bypass via uncanonicalized base path in FileSystemContext::new +**Vulnerability:** The `FileSystemContext::new` method previously accepted a `base_path` and stored it as-is without canonicalization. Later on, `FileSystemContext::secure_path` validated requested paths by joining them with the `base_path` and checking that it `starts_with` the `base_path`, then canonicalizing the base path _just_ to check symlink escapes. If the `base_path` wasn't fully resolved initially, clever manipulation of paths with `..` relative to uncanonicalized base paths combined with symlinks could slip past the starts_with lexical check before the physical check triggered correctly. +**Learning:** An uncanonicalized base directory breaks lexical boundary checks. When checking bounds (`starts_with`) on a potentially dangerous path relative to a base path, the base path itself must be canonicalized _before_ the context is ever used to ensure uniform comparison. +**Prevention:** Canonicalize the absolute base directory upfront during object construction and fail immediately if it cannot be resolved. diff --git a/crates/services/src/lib.rs b/crates/services/src/lib.rs index 4944b30..9152efb 100644 --- a/crates/services/src/lib.rs +++ b/crates/services/src/lib.rs @@ -142,10 +142,15 @@ pub struct FileSystemContext { } impl FileSystemContext { - pub fn new>(base_path: P) -> Self { - Self { - base_path: base_path.as_ref().to_path_buf(), - } + pub fn new>(base_path: P) -> ServiceResult { + let canonical_base = base_path + .as_ref() + .canonicalize() + .map_err(|e| ServiceError::execution_dynamic(format!("Invalid base path: {}", e)))?; + + Ok(Self { + base_path: canonical_base, + }) } /// Lexically validate path to prevent traversal attacks and symlink escapes @@ -310,7 +315,7 @@ mod tests { #[test] fn test_file_system_context_security() { let temp = std::env::temp_dir(); - let ctx = FileSystemContext::new(&temp); + let ctx = FileSystemContext::new(&temp).unwrap(); // Valid paths assert!(ctx.secure_path("test.txt").is_ok()); diff --git a/crates/thread/tests/integration.rs b/crates/thread/tests/integration.rs index 6648b57..48afbde 100644 --- a/crates/thread/tests/integration.rs +++ b/crates/thread/tests/integration.rs @@ -18,5 +18,5 @@ fn test_service_reexports_work() { use thread::services::FileSystemContext; // Just check if we can use the types - let _ctx = FileSystemContext::new("."); + let _ctx = FileSystemContext::new(".").unwrap(); }