diff --git a/poc b/poc new file mode 100755 index 0000000..a5aaea7 Binary files /dev/null and b/poc differ diff --git a/poc.rs b/poc.rs new file mode 100644 index 0000000..095caa3 --- /dev/null +++ b/poc.rs @@ -0,0 +1,57 @@ +use std::path::{Path, PathBuf}; +use std::fs; + +fn canon_or_raw(p: &Path) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in p.components() { + match component { + std::path::Component::ParentDir => { + let can_pop = matches!( + normalized.components().next_back(), + Some(std::path::Component::Normal(_)) + ); + if can_pop { + normalized.pop(); + } else if !normalized.has_root() { + normalized.push(component); + } + } + std::path::Component::CurDir => {} + _ => normalized.push(component), + } + } + normalized +} + +fn secure_canonicalize(path: &Path) -> PathBuf { + let mut p = PathBuf::new(); + for component in path.components() { + p.push(component); + if let Ok(canon) = fs::canonicalize(&p) { + p = canon; + } + } + canon_or_raw(&p) +} + +fn main() { + let temp = std::env::temp_dir(); + let real_allowed = temp.join("allowed_dir_abc"); + fs::create_dir_all(&real_allowed).unwrap(); + let secret = temp.join("secret_dir_xyz"); + fs::create_dir_all(&secret).unwrap(); + + // Attacker makes a symlink from allowed/sym -> target + let symlink = real_allowed.join("sym"); + #[cfg(unix)] + std::os::unix::fs::symlink(&secret, &symlink).unwrap_or(()); + + // Attacker tries to write to allowed/sym/../secret_dir_xyz/new_file + let malicious = symlink.join("..").join("secret_dir_xyz").join("new_file"); + println!("malicious path: {:?}", malicious); + println!("secure_canonicalize: {:?}", secure_canonicalize(&malicious)); + println!("canon_or_raw: {:?}", canon_or_raw(&malicious)); + + let allowed = real_allowed.clone(); + println!("secure_canonicalize(allowed): {:?}", secure_canonicalize(&allowed)); +} diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index 9a32958..3e5eb19 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -67,12 +67,23 @@ fn canon_or_raw(p: &Path) -> PathBuf { normalized } +fn secure_canonicalize(path: &Path) -> PathBuf { + let mut p = PathBuf::new(); + for component in path.components() { + p.push(component); + if let Ok(canon) = fs::canonicalize(&p) { + p = canon; + } + } + canon_or_raw(&p) +} + /// Exact match against an authorized folder, compared canonicalized so a /// symlink or ".." spelling cannot stand in for a different path. A subfolder /// of an authorized folder is not itself authorized. fn path_allowed(candidate: &Path, allowed: &[PathBuf]) -> bool { - let c = canon_or_raw(candidate); - allowed.iter().any(|a| canon_or_raw(a) == c) + let c = secure_canonicalize(candidate); + allowed.iter().any(|a| secure_canonicalize(a) == c) } /// The custom folder recorded in the app's own settings.json, if any. Only diff --git a/test_fix b/test_fix new file mode 100755 index 0000000..e2ef75a Binary files /dev/null and b/test_fix differ diff --git a/test_fix.rs b/test_fix.rs new file mode 100644 index 0000000..d4e4a84 --- /dev/null +++ b/test_fix.rs @@ -0,0 +1,58 @@ +use std::path::{Path, PathBuf}; +use std::fs; + +fn canon_or_raw(p: &Path) -> PathBuf { + let mut normalized = PathBuf::new(); + for component in p.components() { + match component { + std::path::Component::ParentDir => { + let can_pop = matches!( + normalized.components().next_back(), + Some(std::path::Component::Normal(_)) + ); + if can_pop { + normalized.pop(); + } else if !normalized.has_root() { + normalized.push(component); + } + } + std::path::Component::CurDir => {} + _ => normalized.push(component), + } + } + normalized +} + +fn secure_canonicalize(path: &Path) -> PathBuf { + let mut p = PathBuf::new(); + for component in path.components() { + p.push(component); + if let Ok(canon) = fs::canonicalize(&p) { + p = canon; + } + } + // We still need to run canon_or_raw because non-existent parts + // (e.g. trailing ".." or ".") were pushed but not canonicalized by fs::canonicalize + canon_or_raw(&p) +} + +fn path_allowed(candidate: &Path, allowed: &[PathBuf]) -> bool { + let c = secure_canonicalize(candidate); + allowed.iter().any(|a| secure_canonicalize(a) == c) +} + +fn main() { + let temp = std::env::temp_dir(); + let real_allowed = temp.join("allowed_dir_abc"); + fs::create_dir_all(&real_allowed).unwrap(); + let secret = temp.join("secret_dir_xyz"); + fs::create_dir_all(&secret).unwrap(); + + // Attacker makes a symlink from allowed/sym -> target + let symlink = real_allowed.join("sym"); + #[cfg(unix)] + std::os::unix::fs::symlink(&secret, &symlink).unwrap_or(()); + + let malicious = symlink.join("..").join("secret_dir_xyz").join("new_file"); + println!("Does malicious match? {}", path_allowed(&malicious, &[real_allowed.clone()])); +}