From 8a92a0cf1a1f2cd2341cb118e811d5f12177a8c3 Mon Sep 17 00:00:00 2001 From: Falko Galperin <10247603+falko17@users.noreply.github.com> Date: Sat, 12 Apr 2025 03:49:19 +0200 Subject: [PATCH] fix: follow symlinks when searching/archiving (#572) Specifically, this will always follow symlinks when they lead to a path below the dufs root, and will follow other symlinks when `--allow-symlink` is set. I refactored some common functionality out of `zip_dir` and `handle_search_dir` as well. --- src/server.rs | 143 +++++++++++++++++++++++++++++--------------------- 1 file changed, 83 insertions(+), 60 deletions(-) diff --git a/src/server.rs b/src/server.rs index 041bd7e..ef9cf49 100644 --- a/src/server.rs +++ b/src/server.rs @@ -48,7 +48,7 @@ use tokio::{fs, io}; use tokio_util::compat::FuturesAsyncWriteCompatExt; use tokio_util::io::{ReaderStream, StreamReader}; use uuid::Uuid; -use walkdir::WalkDir; +use walkdir::{DirEntry, WalkDir}; use xml::escape::escape_str_pcdata; pub type Request = hyper::Request; @@ -592,36 +592,20 @@ impl Server { } else { let path_buf = path.to_path_buf(); let hidden = Arc::new(self.args.hidden.to_vec()); - let hidden = hidden.clone(); - let running = self.running.clone(); + let search = search.clone(); + let access_paths = access_paths.clone(); - let search_paths = tokio::task::spawn_blocking(move || { - let mut paths: Vec = vec![]; - for dir in access_paths.entry_paths(&path_buf) { - let mut it = WalkDir::new(&dir).into_iter(); - it.next(); - while let Some(Ok(entry)) = it.next() { - if !running.load(atomic::Ordering::SeqCst) { - break; - } - let entry_path = entry.path(); - let base_name = get_file_name(entry_path); - let is_dir = entry.file_type().is_dir(); - if is_hidden(&hidden, base_name, is_dir) { - if is_dir { - it.skip_current_dir(); - } - continue; - } - if !base_name.to_lowercase().contains(&search) { - continue; - } - paths.push(entry_path.to_path_buf()); - } - } - paths - }) + let search_paths = tokio::spawn(collect_dir_entries( + access_paths, + self.running.clone(), + path_buf, + hidden, + self.args.allow_symlink, + self.args.serve_path.clone(), + move |x| get_file_name(x.path()).to_lowercase().contains(&search), + )) .await?; + for search_path in search_paths.into_iter() { if let Ok(Some(item)) = self.to_pathitem(search_path, path.to_path_buf()).await { paths.push(item); @@ -659,6 +643,8 @@ impl Server { let hidden = self.args.hidden.clone(); let running = self.running.clone(); let compression = self.args.compress.to_compression(); + let follow_symlinks = self.args.allow_symlink; + let serve_path = self.args.serve_path.clone(); tokio::spawn(async move { if let Err(e) = zip_dir( &mut writer, @@ -666,11 +652,13 @@ impl Server { access_paths, &hidden, compression, + follow_symlinks, + serve_path, running, ) .await { - error!("Failed to zip {}, {}", path.display(), e); + error!("Failed to zip {}, {e}", path.display()); } }); let reader_stream = ReaderStream::with_capacity(reader, BUF_SIZE); @@ -1639,40 +1627,21 @@ async fn zip_dir( access_paths: AccessPaths, hidden: &[String], compression: Compression, + follow_symlinks: bool, + serve_path: PathBuf, running: Arc, ) -> Result<()> { let mut writer = ZipFileWriter::with_tokio(writer); let hidden = Arc::new(hidden.to_vec()); - let dir_clone = dir.to_path_buf(); - let zip_paths = tokio::task::spawn_blocking(move || { - let mut paths: Vec = vec![]; - for dir in access_paths.entry_paths(&dir_clone) { - let mut it = WalkDir::new(&dir).into_iter(); - it.next(); - while let Some(Ok(entry)) = it.next() { - if !running.load(atomic::Ordering::SeqCst) { - break; - } - let entry_path = entry.path(); - let base_name = get_file_name(entry_path); - let file_type = entry.file_type(); - if is_hidden(&hidden, base_name, file_type.is_dir()) { - if file_type.is_dir() { - it.skip_current_dir(); - } - continue; - } - if entry.path().symlink_metadata().is_err() { - continue; - } - if !file_type.is_file() { - continue; - } - paths.push(entry_path.to_path_buf()); - } - } - paths - }) + let zip_paths = tokio::task::spawn(collect_dir_entries( + access_paths, + running, + dir.to_path_buf(), + hidden, + follow_symlinks, + serve_path, + move |x| x.path().symlink_metadata().is_ok() && x.file_type().is_file(), + )) .await?; for zip_path in zip_paths.into_iter() { let filename = match zip_path.strip_prefix(dir).ok().and_then(|v| v.to_str()) { @@ -1839,3 +1808,57 @@ fn has_query_flag(query_params: &HashMap, name: &str) -> bool { .map(|v| v.is_empty()) .unwrap_or_default() } + +async fn collect_dir_entries( + access_paths: AccessPaths, + running: Arc, + path: PathBuf, + hidden: Arc>, + follow_symlinks: bool, + serve_path: PathBuf, + include_entry: F, +) -> Vec +where + F: Fn(&DirEntry) -> bool, +{ + let mut paths: Vec = vec![]; + for dir in access_paths.entry_paths(&path) { + let mut it = WalkDir::new(&dir).follow_links(true).into_iter(); + it.next(); + while let Some(Ok(entry)) = it.next() { + if !running.load(atomic::Ordering::SeqCst) { + break; + } + let entry_path = entry.path(); + let base_name = get_file_name(entry_path); + let is_dir = entry.file_type().is_dir(); + if is_hidden(&hidden, base_name, is_dir) { + if is_dir { + it.skip_current_dir(); + } + continue; + } + + if !follow_symlinks + && !fs::canonicalize(entry_path) + .await + .ok() + .map(|v| v.starts_with(&serve_path)) + .unwrap_or_default() + { + // We walked outside the server's root. This could only have + // happened if we followed a symlink, and hence we only allow it + // if allow_symlink is enabled, otherwise we skip this entry. + if is_dir { + it.skip_current_dir(); + } + continue; + } + if !include_entry(&entry) { + continue; + } + paths.push(entry_path.to_path_buf()); + } + } + paths +}