aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat (limited to '')
-rw-r--r--src/file_upload.rs41
-rw-r--r--tests/upload_files.rs12
2 files changed, 47 insertions, 6 deletions
diff --git a/src/file_upload.rs b/src/file_upload.rs
index 2319b6a..5009f36 100644
--- a/src/file_upload.rs
+++ b/src/file_upload.rs
@@ -8,7 +8,10 @@ use std::{
use crate::errors::ContextualError;
use crate::listing::{self};
-/// Create future to save file.
+/// Saves file data from a multipart form field (`field`) to `file_path`, optionally overwriting
+/// existing file.
+///
+/// Returns total bytes written to file.
async fn save_file(
field: actix_multipart::Field,
file_path: PathBuf,
@@ -34,7 +37,7 @@ async fn save_file(
Ok(written_len)
}
-/// Create new future to handle file as multipart data.
+/// Handles a single field in a multipart form
async fn handle_multipart(
field: actix_multipart::Field,
path: PathBuf,
@@ -121,6 +124,8 @@ pub async fn upload_file(
/// Guarantee that the path is relative and cannot traverse back to parent directories
/// 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<PathBuf> {
let mut buf = PathBuf::new();
@@ -145,3 +150,35 @@ fn sanitize_path(path: &Path, traverse_hidden: bool) -> Option<PathBuf> {
Some(buf)
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use pretty_assertions::assert_eq;
+ use rstest::rstest;
+
+ #[rstest]
+ #[case("/foo", "foo")]
+ #[case("////foo", "foo")]
+ #[case("C:/foo", if cfg!(windows) { "foo" } else { "C:/foo" })]
+ #[case("../foo", "foo")]
+ #[case("../foo/../bar/abc", "bar/abc")]
+ fn test_sanitize_path(#[case] input: &str, #[case] output: &str) {
+ assert_eq!(
+ sanitize_path(Path::new(input), true).unwrap(),
+ Path::new(output)
+ );
+ assert_eq!(
+ sanitize_path(Path::new(input), false).unwrap(),
+ Path::new(output)
+ );
+ }
+
+ #[rstest]
+ #[case(".foo")]
+ #[case("/.foo")]
+ #[case("foo/.bar/foo")]
+ fn test_sanitize_path_no_hidden_files(#[case] input: &str) {
+ assert_eq!(sanitize_path(Path::new(input), false), None);
+ }
+}
diff --git a/tests/upload_files.rs b/tests/upload_files.rs
index 26750d9..331db1c 100644
--- a/tests/upload_files.rs
+++ b/tests/upload_files.rs
@@ -80,6 +80,10 @@ fn uploading_files_is_prevented(server: TestServer) -> Result<(), Error> {
Ok(())
}
+/// Test for path traversal vulnerability (CWE-22) in both path parameter of query string and in
+/// file name (Content-Disposition)
+///
+/// see: https://github.com/svenstaro/miniserve/issues/518
#[rstest]
#[case("foo", "bar", "foo/bar")]
#[case("/../foo", "bar", "foo/bar")]
@@ -87,13 +91,13 @@ fn uploading_files_is_prevented(server: TestServer) -> Result<(), Error> {
#[case("C:/foo", "C:/bar", if cfg!(windows) { "foo/bar" } else { "C:/foo/C:/bar" })]
#[case(r"C:\foo", r"C:\bar", if cfg!(windows) { "foo/bar" } else { r"C:\foo/C:\bar" })]
#[case(r"\foo", r"\..\bar", if cfg!(windows) { "foo/bar" } else { r"\foo/\..\bar" })]
-fn path_traversal(
+fn prevent_path_traversal_attacks(
#[with(&["-u"])] server: TestServer,
#[case] path: &str,
#[case] filename: &'static str,
#[case] expected: &str,
) -> Result<(), Error> {
- // create test directories
+ // Create test directories
use std::fs::create_dir_all;
create_dir_all(server.path().join("foo")).unwrap();
if !cfg!(windows) {
@@ -132,14 +136,14 @@ fn symlink(#[case] server: TestServer, #[case] ok: bool, tmpdir: TempDir) -> Res
#[cfg(windows)]
use std::os::windows::fs::symlink_dir;
- // create symlink directory "foo" to point outside the root
+ // Create symlink directory "foo" to point outside the root
let (dir, filename) = ("foo", "bar");
symlink_dir(tmpdir.path(), server.path().join(dir)).unwrap();
let full_path = server.path().join(dir).join(filename);
assert!(!full_path.exists());
- // try to upload
+ // Try to upload
let part = multipart::Part::text("this should be uploaded")
.file_name(filename)
.mime_str("text/plain")?;