diff --git a/casket-backend/src/errors.rs b/casket-backend/src/errors.rs index 40505eb..226bb62 100644 --- a/casket-backend/src/errors.rs +++ b/casket-backend/src/errors.rs @@ -2,3 +2,5 @@ pub const ERROR_DETAILS_PATH_TRAVERSAL: &str = "You must not traverse!"; pub const ERROR_DETAILS_INTERNAL_ERROR: &str = "Internal Error! Check the logs of the backend service for details."; pub const ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED: &str = "Absolute paths are not allowed."; +pub const ERROR_DETAILS_NO_NAME_PROVIDED_MULTIPART_FIELD: &str = + "No name provided for multipart form field"; diff --git a/casket-backend/src/files/mod.rs b/casket-backend/src/files/mod.rs index 709ebba..92f6413 100644 --- a/casket-backend/src/files/mod.rs +++ b/casket-backend/src/files/mod.rs @@ -1 +1,121 @@ +use crate::errors; +use axum::http::StatusCode; +use problem_details::ProblemDetails; +use std::path::{Component, Path, PathBuf}; + +pub mod list; pub mod upload; + +fn build_system_path( + base_directory: &Path, + user_id: &str, + user_path: &PathBuf, +) -> Result { + if user_path.is_absolute() { + Err( + ProblemDetails::from_status_code(StatusCode::BAD_REQUEST).with_detail( + errors::ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED.to_owned() + + format!(" Provided Path: {:?}", user_path).as_str(), + ), + ) + } else { + sanitize_path(&base_directory.join(user_id).join(user_path)) + } +} + +pub fn sanitize_path(path: &Path) -> Result { + let mut ret = PathBuf::new(); + for component in path.components().peekable() { + match component { + Component::Prefix(..) => unreachable!(), + Component::RootDir => { + ret.push(Component::RootDir); + } + Component::CurDir => {} + Component::ParentDir => { + return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(errors::ERROR_DETAILS_PATH_TRAVERSAL)); + } + Component::Normal(c) => { + ret.push(c); + } + } + } + Ok(ret) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sanitize_path_success() { + let cases = &[ + ("", ""), + (".", ""), + (".////./.", ""), + ("/", "/"), + ("/foo/bar", "/foo/bar"), + ("/foo/bar/", "/foo/bar"), + ("/foo/bar/./././///", "/foo/bar"), + ("foo/bar", "foo/bar"), + ("foo/bar/", "foo/bar"), + ("foo/bar/./././///", "foo/bar"), + ]; + for (input, expected) in cases { + let actual = sanitize_path(&PathBuf::from(input)); + assert_eq!(actual, Ok(PathBuf::from(expected)), "input: {input}"); + } + } + + #[test] + fn test_sanitize_path_error() { + let cases = &[ + "..", + "/../", + "../../foo/bar", + "../../foo/bar/", + "../../foo/bar/./././///", + "../../foo/bar/..", + "../../foo/bar/../..", + "../../foo/bar/../../..", + "/foo/bar/../../..", + "/foo/bar/../..", + "/foo/bar/..", + ]; + for input in cases { + let actual = sanitize_path(&PathBuf::from(input)); + assert_eq!( + actual, + Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(errors::ERROR_DETAILS_PATH_TRAVERSAL)), + "input: {input}" + ); + } + } + + #[test] + fn test_build_system_path_success() { + let input_path = PathBuf::from("tmp/blub"); + let user_id = "bla"; + assert_eq!( + build_system_path(&PathBuf::from("/tmp/bla"), user_id, &input_path).unwrap(), + PathBuf::from("/tmp/bla/bla/tmp/blub") + ); + } + + #[test] + fn test_build_system_path_error_with_absolute_user_path_returns_error() { + let input_path = PathBuf::from("tmp/blub"); + let user_id = "bla"; + assert_eq!( + build_system_path(&PathBuf::from("/tmp/bla"), user_id, &input_path), + Err( + ProblemDetails::from_status_code(StatusCode::BAD_REQUEST).with_detail( + errors::ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED.to_owned() + + format!(" Provided Path: {:?}", input_path).as_str() + ) + ) + ); + } +} diff --git a/casket-backend/src/files/upload.rs b/casket-backend/src/files/upload.rs index 724eb9d..fe20a40 100644 --- a/casket-backend/src/files/upload.rs +++ b/casket-backend/src/files/upload.rs @@ -1,3 +1,4 @@ +use crate::files::build_system_path; use crate::{errors, AppState}; use axum::body::Bytes; use axum::debug_handler; @@ -22,10 +23,14 @@ pub async fn handle_file_uploads( axum::extract::Path(user_id): axum::extract::Path, mut multipart: Multipart, ) -> Result<(), ProblemDetails> { - while let Some(field) = multipart.next_field().await.unwrap() { - handle_single_file_upload(&state, &user_id, field).await?; + loop { + let next = multipart.next_field().await; + match next { + Err(error) => return Err(to_internal_error(error)), + Ok(Some(field)) => handle_single_file_upload(&state, &user_id, field).await?, + Ok(None) => return Ok(()), + } } - Ok(()) } // clippy behaves weird. When removing the lifetime the compilation fails since the Field type has @@ -36,11 +41,17 @@ async fn handle_single_file_upload<'field_lifetime>( user_id: &str, field: Field<'field_lifetime>, ) -> Result<(), ProblemDetails> { - let path = field.name().unwrap().to_string(); - let filesystem_path = build_system_path(&state.config.files.directory, user_id, &path)?; - save_file(filesystem_path, map_error_to_io_error(field)) - .await - .map_err(to_internal_error) + match field.name() { + None | Some("") => Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) + .with_detail(errors::ERROR_DETAILS_NO_NAME_PROVIDED_MULTIPART_FIELD)), + Some(field_name) => { + let path = PathBuf::from(field_name); + let filesystem_path = build_system_path(&state.config.files.directory, user_id, &path)?; + save_file(filesystem_path, map_error_to_io_error(field)) + .await + .map_err(to_internal_error) + } + } } pub async fn save_file( @@ -64,118 +75,3 @@ fn to_internal_error(error: impl Debug) -> ProblemDetails { fn map_error_to_io_error(field: Field) -> MapErr Error> { field.map_err(|err| Error::new(io::ErrorKind::Other, err)) } - -fn build_system_path( - base_directory: &Path, - user_id: &str, - path: &str, -) -> Result { - let user_path = PathBuf::from(path); - if user_path.is_absolute() { - Err( - ProblemDetails::from_status_code(StatusCode::BAD_REQUEST).with_detail( - errors::ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED.to_owned() - + format!(" Provided Path: {path}").as_str(), - ), - ) - } else { - sanitize_path(&base_directory.join(user_id).join(path)) - } -} - -pub fn sanitize_path(path: &Path) -> Result { - let mut ret = PathBuf::new(); - for component in path.components().peekable() { - match component { - Component::Prefix(..) => unreachable!(), - Component::RootDir => { - ret.push(Component::RootDir); - } - Component::CurDir => {} - Component::ParentDir => { - return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) - .with_detail(errors::ERROR_DETAILS_PATH_TRAVERSAL)); - } - Component::Normal(c) => { - ret.push(c); - } - } - } - Ok(ret) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_sanitize_path_success() { - let cases = &[ - ("", ""), - (".", ""), - (".////./.", ""), - ("/", "/"), - ("/foo/bar", "/foo/bar"), - ("/foo/bar/", "/foo/bar"), - ("/foo/bar/./././///", "/foo/bar"), - ("foo/bar", "foo/bar"), - ("foo/bar/", "foo/bar"), - ("foo/bar/./././///", "foo/bar"), - ]; - for (input, expected) in cases { - let actual = sanitize_path(&PathBuf::from(input)); - assert_eq!(actual, Ok(PathBuf::from(expected)), "input: {input}"); - } - } - - #[test] - fn test_sanitize_path_error() { - let cases = &[ - "..", - "/../", - "../../foo/bar", - "../../foo/bar/", - "../../foo/bar/./././///", - "../../foo/bar/..", - "../../foo/bar/../..", - "../../foo/bar/../../..", - "/foo/bar/../../..", - "/foo/bar/../..", - "/foo/bar/..", - ]; - for input in cases { - let actual = sanitize_path(&PathBuf::from(input)); - assert_eq!( - actual, - Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST) - .with_detail(errors::ERROR_DETAILS_PATH_TRAVERSAL)), - "input: {input}" - ); - } - } - - #[test] - fn test_build_system_path_success() { - let input_path = "tmp/blub"; - let user_id = "bla"; - assert_eq!( - build_system_path(&PathBuf::from("/tmp/bla"), user_id, input_path).unwrap(), - PathBuf::from("/tmp/bla/bla/tmp/blub") - ); - } - - #[test] - fn test_build_system_path_error_with_absolute_user_path_returns_error() { - let input_path = "tmp/blub"; - let user_id = "bla"; - assert_eq!( - build_system_path(&PathBuf::from("/tmp/bla"), user_id, input_path), - Err( - ProblemDetails::from_status_code(StatusCode::BAD_REQUEST).with_detail( - errors::ERROR_DETAILS_ABSOLUTE_PATH_NOT_ALLOWED.to_owned() - + format!(" Provided Path: {input_path}").as_str() - ) - ) - ); - } -}