diff --git a/Cargo.lock b/Cargo.lock index ec3b7ec..e6c470a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1938,6 +1938,12 @@ version = "1.0.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c" +[[package]] +name = "path-clean" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "17359afc20d7ab31fdb42bb844c8b3bb1dabd7dcf7e68428492da7f16966fcef" + [[package]] name = "pathdiff" version = "0.2.1" @@ -2425,6 +2431,7 @@ dependencies = [ "infer", "lazy-regex", "mime", + "path-clean", "petname", "rand", "regex", diff --git a/Cargo.toml b/Cargo.toml index bd1960d..571e11a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ tokio = { version = "1.35.1", optional = true } tracing = "0.1.40" tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } uts2ts = "0.4.1" +path-clean = "1.0.1" [dependencies.config] version = "0.13.4" diff --git a/src/main.rs b/src/main.rs index f519c46..14705a7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -78,7 +78,7 @@ fn setup(config_folder: &Path) -> IoResult<(Data>, ServerConfig, // Create necessary directories. fs::create_dir_all(&server_config.upload_path)?; for paste_type in &[PasteType::Url, PasteType::Oneshot, PasteType::OneshotUrl] { - fs::create_dir_all(paste_type.get_path(&server_config.upload_path))?; + fs::create_dir_all(paste_type.get_path(&server_config.upload_path)?)?; } // Set up a watcher for the configuration file changes. diff --git a/src/paste.rs b/src/paste.rs index 03b94f5..373922f 100644 --- a/src/paste.rs +++ b/src/paste.rs @@ -58,12 +58,12 @@ impl PasteType { } /// Returns the given path with [`directory`](Self::get_dir) adjoined. - pub fn get_path(&self, path: &Path) -> PathBuf { + pub fn get_path(&self, path: &Path) -> IoResult { let dir = self.get_dir(); if dir.is_empty() { - path.to_path_buf() + Ok(path.to_path_buf()) } else { - path.join(dir) + util::safe_path_join(path, Path::new(&dir)) } } @@ -122,10 +122,9 @@ impl Paste { if let Some(handle_spaces_config) = config.server.handle_spaces { file_name = handle_spaces_config.process_filename(&file_name); } - let mut path = self - .type_ - .get_path(&config.server.upload_path) - .join(&file_name); + + let mut path = + util::safe_path_join(self.type_.get_path(&config.server.upload_path)?, &file_name)?; let mut parts: Vec<&str> = file_name.split('.').collect(); let mut dotfile = false; let mut lower_bound = 1; @@ -261,10 +260,8 @@ impl Paste { file_name = random_text; } } - let mut path = self - .type_ - .get_path(&config.server.upload_path) - .join(&file_name); + let mut path = + util::safe_path_join(self.type_.get_path(&config.server.upload_path)?, &file_name)?; if let Some(timestamp) = expiry_date { path.set_file_name(format!("{file_name}.{timestamp}")); } @@ -430,7 +427,11 @@ mod tests { fs::remove_file(file_name)?; for paste_type in &[PasteType::Url, PasteType::Oneshot] { - fs::create_dir_all(paste_type.get_path(&config.server.upload_path))?; + fs::create_dir_all( + paste_type + .get_path(&config.server.upload_path) + .expect("Bad upload path"), + )?; } config.paste.random_url = None; @@ -442,6 +443,7 @@ mod tests { let file_name = paste.store_file("test.file", Some(expiry_date), None, &config)?; let file_path = PasteType::Oneshot .get_path(&config.server.upload_path) + .expect("Bad upload path") .join(format!("{file_name}.{expiry_date}")); assert_eq!("test", fs::read_to_string(&file_path)?); fs::remove_file(file_path)?; @@ -458,6 +460,7 @@ mod tests { let file_name = paste.store_url(None, &config)?; let file_path = PasteType::Url .get_path(&config.server.upload_path) + .expect("Bad upload path") .join(&file_name); assert_eq!(url, fs::read_to_string(&file_path)?); fs::remove_file(file_path)?; @@ -485,6 +488,7 @@ mod tests { .await?; let file_path = PasteType::RemoteFile .get_path(&config.server.upload_path) + .expect("Bad upload path") .join(file_name); assert_eq!( "8c712905b799905357b8202d0cb7a244cefeeccf7aa5eb79896645ac50158ffa", @@ -493,7 +497,11 @@ mod tests { fs::remove_file(file_path)?; for paste_type in &[PasteType::Url, PasteType::Oneshot] { - fs::remove_dir(paste_type.get_path(&config.server.upload_path))?; + fs::remove_dir( + paste_type + .get_path(&config.server.upload_path) + .expect("Bad upload path"), + )?; } Ok(()) diff --git a/src/server.rs b/src/server.rs index 1e585a1..eff2f73 100644 --- a/src/server.rs +++ b/src/server.rs @@ -4,7 +4,7 @@ use crate::file::Directory; use crate::header::{self, ContentDisposition}; use crate::mime as mime_util; use crate::paste::{Paste, PasteType}; -use crate::util; +use crate::util::{self, safe_path_join}; use actix_files::NamedFile; use actix_multipart::Multipart; use actix_web::http::StatusCode; @@ -89,12 +89,11 @@ async fn serve( let config = config .read() .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; - let path = config.server.upload_path.join(&*file); - let mut path = util::glob_match_file(path)?; + let mut path = util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?)?; let mut paste_type = PasteType::File; if !path.exists() || path.is_dir() { for type_ in &[PasteType::Url, PasteType::Oneshot, PasteType::OneshotUrl] { - let alt_path = type_.get_path(&config.server.upload_path).join(&*file); + let alt_path = safe_path_join(type_.get_path(&config.server.upload_path)?, &*file)?; let alt_path = util::glob_match_file(alt_path)?; if alt_path.exists() || path.file_name().and_then(|v| v.to_str()) == Some(&type_.get_dir()) @@ -159,8 +158,7 @@ async fn delete( let config = config .read() .map_err(|_| error::ErrorInternalServerError("cannot acquire config"))?; - let path = config.server.upload_path.join(&*file); - let path = util::glob_match_file(path)?; + let path = util::glob_match_file(safe_path_join(&config.server.upload_path, &*file)?)?; if !path.is_file() || !path.exists() { return Err(error::ErrorNotFound("file is not found or expired :(\n")); } @@ -1087,7 +1085,9 @@ mod tests { ) .await; - let url_upload_path = PasteType::Url.get_path(&config.server.upload_path); + let url_upload_path = PasteType::Url + .get_path(&config.server.upload_path) + .expect("Bad upload path"); fs::create_dir_all(&url_upload_path)?; let response = test::call_service( @@ -1125,7 +1125,9 @@ mod tests { ) .await; - let oneshot_upload_path = PasteType::Oneshot.get_path(&config.server.upload_path); + let oneshot_upload_path = PasteType::Oneshot + .get_path(&config.server.upload_path) + .expect("Bad upload path"); fs::create_dir_all(&oneshot_upload_path)?; let file_name = "oneshot.txt"; @@ -1185,7 +1187,9 @@ mod tests { ) .await; - let url_upload_path = PasteType::OneshotUrl.get_path(&config.server.upload_path); + let url_upload_path = PasteType::OneshotUrl + .get_path(&config.server.upload_path) + .expect("Bad upload path"); fs::create_dir_all(&url_upload_path)?; let response = test::call_service( diff --git a/src/util.rs b/src/util.rs index ebc3c9d..11bac07 100644 --- a/src/util.rs +++ b/src/util.rs @@ -2,6 +2,7 @@ use crate::paste::PasteType; use actix_web::{error, Error as ActixError}; use glob::glob; use lazy_regex::{lazy_regex, Lazy, Regex}; +use path_clean::PathClean; use ring::digest::{Context, SHA256}; use std::fmt::Write; use std::io::{BufReader, Read}; @@ -64,7 +65,8 @@ pub fn get_expired_files(base_path: &Path) -> Vec { PasteType::OneshotUrl, ] .into_iter() - .filter_map(|v| glob(&v.get_path(base_path).join("*.[0-9]*").to_string_lossy()).ok()) + .filter_map(|v| v.get_path(base_path).ok()) + .filter_map(|v| glob(&v.join("*.[0-9]*").to_string_lossy()).ok()) .flat_map(|glob| glob.filter_map(|v| v.ok()).collect::>()) .filter(|path| { if let Some(extension) = path @@ -108,6 +110,27 @@ pub fn sha256_digest(input: R) -> Result { })?) } +/// Joins the paths whilst ensuring the path doesn't drastically change. +/// `base` is assumed to be a trusted value. +pub fn safe_path_join, P: AsRef>(base: B, part: P) -> IoResult { + let new_path = base.as_ref().join(part).clean(); + + let cleaned_base = base.as_ref().clean(); + + if !new_path.starts_with(cleaned_base) { + return Err(IoError::new( + IoErrorKind::InvalidData, + format!( + "{} is outside of {}", + new_path.display(), + base.as_ref().display() + ), + )); + } + + Ok(new_path) +} + #[cfg(test)] mod tests { use super::*; @@ -170,4 +193,24 @@ mod tests { assert_eq!(Vec::::new(), get_expired_files(¤t_dir)); Ok(()) } + + #[test] + fn test_safe_join_path() { + assert_eq!(safe_path_join("/foo", "bar").ok(), Some("/foo/bar".into())); + assert_eq!(safe_path_join("/", "bar").ok(), Some("/bar".into())); + assert_eq!(safe_path_join("/", "././bar").ok(), Some("/bar".into())); + assert_eq!( + safe_path_join("/foo/bar", "baz/").ok(), + Some("/foo/bar/baz/".into()) + ); + assert_eq!( + safe_path_join("/foo/bar/../", "baz").ok(), + Some("/foo/baz".into()) + ); + + assert!(safe_path_join("/foo", "/foobar").is_err()); + assert!(safe_path_join("/foo", "/bar").is_err()); + assert!(safe_path_join("/foo/bar", "..").is_err()); + assert!(safe_path_join("/foo/bar", "../").is_err()); + } }