From 2662c59fcffe1b62e019b08d1e22c1cd5c741066 Mon Sep 17 00:00:00 2001 From: Jonas Diemer Date: Tue, 2 Aug 2022 15:02:09 +0200 Subject: Added option restrict-upload-dir --- src/file_upload.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 6643c68..747d0de 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -171,6 +171,19 @@ pub async fn upload_file( ContextualError::IoError("Failed to resolve path served by miniserve".to_string(), e) })?; + + // Disallow paths outside of restricted directories + // TODO: Probably not the most rust-ic style... + if !conf.restrict_upload_dir.is_empty() { + let upl_path = upload_path.clone().into_os_string().into_string().unwrap(); + + if !(conf.restrict_upload_dir.contains(&upl_path)){ + // not good + return Err(ContextualError::InvalidPathError("Not allowed to upload to this path".to_string())); + } + } + + // Disallow the target path to go outside of the served directory // The target directory shouldn't be canonicalized when it gets passed to // handle_multipart so that it can check for symlinks if needed -- cgit v1.2.3 From 455abe23d0fd2114f7836694502892990180577d Mon Sep 17 00:00:00 2001 From: Jonas Diemer Date: Wed, 3 Aug 2022 13:02:21 +0200 Subject: Switched to use of PathBuf, fixed for subdirs --- src/file_upload.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 747d0de..56112f3 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -175,10 +175,15 @@ pub async fn upload_file( // Disallow paths outside of restricted directories // TODO: Probably not the most rust-ic style... if !conf.restrict_upload_dir.is_empty() { - let upl_path = upload_path.clone().into_os_string().into_string().unwrap(); + let mut upload_allowed = false; + for restricted_dir in conf.restrict_upload_dir.iter() { + if upload_path.starts_with(restricted_dir) { + upload_allowed = true; + break; + } + } - if !(conf.restrict_upload_dir.contains(&upl_path)){ - // not good + if !upload_allowed { return Err(ContextualError::InvalidPathError("Not allowed to upload to this path".to_string())); } } -- cgit v1.2.3 From e5804835e491b77625541b025669efa5524d8102 Mon Sep 17 00:00:00 2001 From: Jonas Diemer Date: Wed, 3 Aug 2022 13:32:57 +0200 Subject: cleaned up code using any() --- src/file_upload.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 56112f3..e5f6173 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -174,21 +174,15 @@ pub async fn upload_file( // Disallow paths outside of restricted directories // TODO: Probably not the most rust-ic style... - if !conf.restrict_upload_dir.is_empty() { - let mut upload_allowed = false; - for restricted_dir in conf.restrict_upload_dir.iter() { - if upload_path.starts_with(restricted_dir) { - upload_allowed = true; - break; - } - } + let upload_allowed = conf.restrict_upload_dir.is_empty() || + conf.restrict_upload_dir.iter().any(|s| upload_path.starts_with(s)); - if !upload_allowed { - return Err(ContextualError::InvalidPathError("Not allowed to upload to this path".to_string())); - } + if !(upload_allowed) { + return Err(ContextualError::InvalidPathError("Not allowed to upload to this path".to_string())); } + // Disallow the target path to go outside of the served directory // The target directory shouldn't be canonicalized when it gets passed to // handle_multipart so that it can check for symlinks if needed -- cgit v1.2.3 From 550ae0151c1dadc6c1f00df300d88528c29fbf49 Mon Sep 17 00:00:00 2001 From: Jonas Diemer Date: Thu, 4 Aug 2022 11:20:37 +0200 Subject: Renamed option for more clarity --- src/file_upload.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index e5f6173..231999f 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -173,9 +173,8 @@ pub async fn upload_file( // Disallow paths outside of restricted directories - // TODO: Probably not the most rust-ic style... - let upload_allowed = conf.restrict_upload_dir.is_empty() || - conf.restrict_upload_dir.iter().any(|s| upload_path.starts_with(s)); + let upload_allowed = conf.allowed_upload_dir.is_empty() || + conf.allowed_upload_dir.iter().any(|s| upload_path.starts_with(s)); if !(upload_allowed) { return Err(ContextualError::InvalidPathError("Not allowed to upload to this path".to_string())); -- cgit v1.2.3 From 54738df913e7b41967b6e56b871ba0a6bf32a190 Mon Sep 17 00:00:00 2001 From: Jonas Diemer Date: Thu, 4 Aug 2022 11:11:47 +0200 Subject: Update src/file_upload.rs Co-authored-by: Sven-Hendrik Haase --- src/file_upload.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 231999f..68371e8 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -176,7 +176,7 @@ pub async fn upload_file( let upload_allowed = conf.allowed_upload_dir.is_empty() || conf.allowed_upload_dir.iter().any(|s| upload_path.starts_with(s)); - if !(upload_allowed) { + if !upload_allowed { return Err(ContextualError::InvalidPathError("Not allowed to upload to this path".to_string())); } -- cgit v1.2.3 From ed925eaa2985b4cc9051371119591ed329b1a57b Mon Sep 17 00:00:00 2001 From: Jonas Diemer Date: Sun, 14 Aug 2022 20:58:40 +0200 Subject: clarity of comment Co-authored-by: Sven-Hendrik Haase --- src/file_upload.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 68371e8..c6e7ac6 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -172,7 +172,7 @@ pub async fn upload_file( })?; - // Disallow paths outside of restricted directories + // Disallow paths outside of allowed directories let upload_allowed = conf.allowed_upload_dir.is_empty() || conf.allowed_upload_dir.iter().any(|s| upload_path.starts_with(s)); -- cgit v1.2.3 From 5404e4fcb513bd8bf355e730aa37546b16164cad Mon Sep 17 00:00:00 2001 From: Jonas Diemer Date: Wed, 17 Aug 2022 10:28:11 +0200 Subject: sanitize allowed upload paths for cases like ./dir --- src/file_upload.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index c6e7ac6..4d4f225 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -218,7 +218,7 @@ pub async fn upload_file( /// and optionally prevent traversing hidden directories. /// /// See the unit tests tests::test_sanitize_path* for examples -fn sanitize_path(path: &Path, traverse_hidden: bool) -> Option { +pub fn sanitize_path(path: &Path, traverse_hidden: bool) -> Option { let mut buf = PathBuf::new(); for comp in path.components() { -- cgit v1.2.3 From d905b68ca93c42769c3ebddf472a2916dc75b012 Mon Sep 17 00:00:00 2001 From: Jonas Diemer Date: Wed, 17 Aug 2022 10:28:46 +0200 Subject: cargo fmt --- src/file_upload.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 4d4f225..0232c7e 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -171,17 +171,19 @@ pub async fn upload_file( ContextualError::IoError("Failed to resolve path served by miniserve".to_string(), e) })?; - // Disallow paths outside of allowed directories - let upload_allowed = conf.allowed_upload_dir.is_empty() || - conf.allowed_upload_dir.iter().any(|s| upload_path.starts_with(s)); + let upload_allowed = conf.allowed_upload_dir.is_empty() + || conf + .allowed_upload_dir + .iter() + .any(|s| upload_path.starts_with(s)); if !upload_allowed { - return Err(ContextualError::InvalidPathError("Not allowed to upload to this path".to_string())); + return Err(ContextualError::InvalidPathError( + "Not allowed to upload to this path".to_string(), + )); } - - // Disallow the target path to go outside of the served directory // The target directory shouldn't be canonicalized when it gets passed to // handle_multipart so that it can check for symlinks if needed -- cgit v1.2.3 From 20a055dd82b009e94b1aa681cc4329f17e552f44 Mon Sep 17 00:00:00 2001 From: Jonas Diemer Date: Mon, 19 Sep 2022 16:43:50 +0200 Subject: Return 403 instead of 500 for upload errs --- src/file_upload.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'src/file_upload.rs') diff --git a/src/file_upload.rs b/src/file_upload.rs index 0232c7e..cf214b8 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -179,9 +179,7 @@ pub async fn upload_file( .any(|s| upload_path.starts_with(s)); if !upload_allowed { - return Err(ContextualError::InvalidPathError( - "Not allowed to upload to this path".to_string(), - )); + return Err(ContextualError::UploadForbiddenError); } // Disallow the target path to go outside of the served directory -- cgit v1.2.3