diff options
| -rw-r--r-- | CHANGELOG.md | 6 | ||||
| -rw-r--r-- | Cargo.lock | 4 | ||||
| -rw-r--r-- | Cargo.toml | 3 | ||||
| -rw-r--r-- | src/args.rs | 17 | ||||
| -rw-r--r-- | src/config.rs | 18 | ||||
| -rw-r--r-- | src/errors.rs | 6 | ||||
| -rw-r--r-- | src/file_upload.rs | 13 | ||||
| -rw-r--r-- | src/listing.rs | 17 | ||||
| -rw-r--r-- | src/renderer.rs | 10 | ||||
| -rw-r--r-- | tests/readme.rs | 22 | ||||
| -rw-r--r-- | tests/upload_files.rs | 91 |
11 files changed, 177 insertions, 30 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index f0be364..dc1f0ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] - ReleaseDate - Faster QR code generation [#848](https://github.com/svenstaro/miniserve/pull/848/files) (thanks @cyqsimon) +- Make `--readme` support not only `README.md` but also `README` and `README.txt` rendered as + plaintext [#911](https://github.com/svenstaro/miniserve/pull/911) (thanks @Atreyagaurav) +- Change `-u/--upload-files` slightly in the sense that it can now either be provided by itself as + before or receive a file path to restrict uploading to only that path. Can be provided multiple + times for multiple allowed paths [#858](https://github.com/svenstaro/miniserve/pull/858) (thanks + @jonasdiemer) ## [0.21.0] - 2022-09-15 - Fix bug where static files would be served incorrectly when using `--random-route` [#835](https://github.com/svenstaro/miniserve/pull/835) (thanks @solarknight) @@ -839,9 +839,9 @@ dependencies = [ [[package]] name = "fast_qr" -version = "0.3.1" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b798dfd6e29b85c0bcf434272db4cde0100ab4d82c5db0a4f422e77b30d0b4e4" +checksum = "c6ca492f7dae5b7705f8997b042fcf7a665addb13181908d1fa7cb6d9bc1c0e6" dependencies = [ "wasm-bindgen", ] @@ -32,7 +32,7 @@ clap = { version = "3.2", features = ["derive", "cargo", "wrap_help"] } clap_complete = "3.2.3" clap_mangen = "0.1" comrak = "0.14.0" -fast_qr = "0.3.1" +fast_qr = "0.5.1" futures = "0.3" get_if_addrs = "0.5" hex = "0.4" @@ -45,6 +45,7 @@ mime = "0.3" nanoid = "0.4" percent-encoding = "2" port_check = "0.1" +regex = "1" rustls = { version = "0.20", optional = true } rustls-pemfile = { version = "1.0", optional = true } serde = { version = "1", features = ["derive"] } diff --git a/src/args.rs b/src/args.rs index a8718bd..8757bc8 100644 --- a/src/args.rs +++ b/src/args.rs @@ -107,23 +107,28 @@ pub struct CliArgs { #[clap(short = 'q', long = "qrcode")] pub qrcode: bool, - /// Enable file uploading - #[clap(short = 'u', long = "upload-files")] - pub file_upload: bool, + /// Enable file uploading (and optionally specify for which directory) + #[clap(short = 'u', long = "upload-files", value_hint = ValueHint::FilePath, min_values = 0)] + pub allowed_upload_dir: Option<Vec<PathBuf>>, /// Enable creating directories - #[clap(short = 'U', long = "mkdir", requires = "file-upload")] + #[clap(short = 'U', long = "mkdir", requires = "allowed-upload-dir")] pub mkdir_enabled: bool, /// Specify uploadable media types - #[clap(arg_enum, short = 'm', long = "media-type", requires = "file-upload")] + #[clap( + arg_enum, + short = 'm', + long = "media-type", + requires = "allowed-upload-dir" + )] pub media_type: Option<Vec<MediaType>>, /// Directly specify the uploadable media type expression #[clap( short = 'M', long = "raw-media-type", - requires = "file-upload", + requires = "allowed-upload-dir", conflicts_with = "media-type" )] pub media_type_raw: Option<String>, diff --git a/src/config.rs b/src/config.rs index 5bcbd62..7ca0693 100644 --- a/src/config.rs +++ b/src/config.rs @@ -16,6 +16,7 @@ use rustls_pemfile as pemfile; use crate::{ args::{CliArgs, MediaType}, auth::RequiredAuth, + file_upload::sanitize_path, }; /// Possible characters for random routes @@ -87,6 +88,9 @@ pub struct MiniserveConfig { /// Enable file upload pub file_upload: bool, + /// List of allowed upload directories + pub allowed_upload_dir: Vec<String>, + /// HTML accept attribute value pub uploadable_media_type: Option<String>, @@ -247,7 +251,19 @@ impl MiniserveConfig { overwrite_files: args.overwrite_files, show_qrcode: args.qrcode, mkdir_enabled: args.mkdir_enabled, - file_upload: args.file_upload, + file_upload: args.allowed_upload_dir.is_some(), + allowed_upload_dir: args + .allowed_upload_dir + .unwrap_or_default() + .iter() + .map(|x| { + sanitize_path(x, false) + .unwrap() + .to_str() + .unwrap() + .replace('\\', "/") + }) + .collect(), uploadable_media_type, tar_enabled: args.enable_tar, tar_gz_enabled: args.enable_tar_gz, diff --git a/src/errors.rs b/src/errors.rs index b2ed459..06569d3 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -22,6 +22,10 @@ pub enum ContextualError { #[error("File already exists, and the overwrite_files option has not been set")] DuplicateFileError, + /// Upload not allowed + #[error("Upload not allowed to this directory")] + UploadForbiddenError, + /// Any error related to an invalid path (failed to retrieve entry name, unexpected entry type, etc) #[error("Invalid path\ncaused by: {0}")] InvalidPathError(String), @@ -88,6 +92,8 @@ impl ResponseError for ContextualError { Self::InsufficientPermissionsError(_) => StatusCode::FORBIDDEN, Self::InvalidHttpCredentials => StatusCode::UNAUTHORIZED, Self::InvalidHttpRequestError(_) => StatusCode::BAD_REQUEST, + Self::DuplicateFileError => StatusCode::FORBIDDEN, + Self::UploadForbiddenError => StatusCode::FORBIDDEN, _ => StatusCode::INTERNAL_SERVER_ERROR, } } diff --git a/src/file_upload.rs b/src/file_upload.rs index 6643c68..cf214b8 100644 --- a/src/file_upload.rs +++ b/src/file_upload.rs @@ -171,6 +171,17 @@ 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)); + + if !upload_allowed { + return Err(ContextualError::UploadForbiddenError); + } + // 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 @@ -207,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<PathBuf> { +pub fn sanitize_path(path: &Path, traverse_hidden: bool) -> Option<PathBuf> { let mut buf = PathBuf::new(); for comp in path.components() { diff --git a/src/listing.rs b/src/listing.rs index 7477599..fbee8de 100644 --- a/src/listing.rs +++ b/src/listing.rs @@ -9,6 +9,7 @@ use actix_web::{HttpMessage, HttpRequest, HttpResponse}; use bytesize::ByteSize; use comrak::{markdown_to_html, ComrakOptions}; use percent_encoding::{percent_decode_str, utf8_percent_encode}; +use regex::Regex; use serde::Deserialize; use strum_macros::{Display, EnumString}; @@ -224,6 +225,7 @@ pub fn directory_listing( let mut entries: Vec<Entry> = Vec::new(); let mut readme: Option<(String, String)> = None; + let readme_rx: Regex = Regex::new("^readme([.](md|txt))?$").unwrap(); for entry in dir.path.read_dir()? { if dir.is_visible(&entry) || conf.show_hidden { @@ -274,13 +276,18 @@ pub fn directory_listing( last_modification_date, symlink_dest, )); - if conf.readme && file_name.to_lowercase() == "readme.md" { + if conf.readme && readme_rx.is_match(&file_name.to_lowercase()) { + let ext = file_name.split('.').last().unwrap().to_lowercase(); readme = Some(( file_name.to_string(), - markdown_to_html( - &std::fs::read_to_string(entry.path())?, - &ComrakOptions::default(), - ), + if ext == "md" { + markdown_to_html( + &std::fs::read_to_string(entry.path())?, + &ComrakOptions::default(), + ) + } else { + format!("<pre>{}</pre>", &std::fs::read_to_string(entry.path())?) + }, )); } } diff --git a/src/renderer.rs b/src/renderer.rs index c8958fe..6aad5bd 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -40,6 +40,12 @@ pub fn page( let title_path = breadcrumbs_to_path_string(breadcrumbs); + let upload_allowed = conf.allowed_upload_dir.is_empty() + || conf + .allowed_upload_dir + .iter() + .any(|x| encoded_dir.starts_with(&format!("/{}", x))); + html! { (DOCTYPE) html { @@ -120,7 +126,7 @@ pub fn page( } } div.toolbar_box_group { - @if conf.file_upload { + @if conf.file_upload && upload_allowed { div.toolbar_box { form id="file_submit" action=(upload_action) method="POST" enctype="multipart/form-data" { p { "Select a file to upload or drag it anywhere into the window" } @@ -235,7 +241,7 @@ fn qr_code_svg(url: impl AsRef<str>, margin: usize) -> Result<String, QRCodeErro let qr = QRBuilder::new(url.as_ref().into()) .ecl(consts::QR_EC_LEVEL) .build()?; - let svg = SvgBuilder::new().margin(margin).build_qr(qr); + let svg = SvgBuilder::default().margin(margin).to_str(&qr); Ok(svg) } diff --git a/tests/readme.rs b/tests/readme.rs index 56a3afd..3fcbe29 100644 --- a/tests/readme.rs +++ b/tests/readme.rs @@ -64,7 +64,11 @@ fn show_root_readme_contents( case("readme.md"), case("README.md"), case("README.MD"), - case("ReAdMe.Md") + case("ReAdMe.Md"), + case("Readme.txt"), + case("README.txt"), + case("README"), + case("ReAdMe") )] fn show_nested_readme_contents( #[with(&["--readme"])] server: TestServer, @@ -113,13 +117,11 @@ fn assert_readme_contents(parsed_dom: &Document, filename: &str) { .find(Attr("id", "readme-contents")) .next() .is_some()); - assert!( - parsed_dom - .find(Attr("id", "readme-contents")) - .next() - .unwrap() - .text() - .trim() - == format!("Contents of {}", filename) - ); + assert!(parsed_dom + .find(Attr("id", "readme-contents")) + .next() + .unwrap() + .text() + .trim() + .contains(&format!("Contents of {}", filename))); } diff --git a/tests/upload_files.rs b/tests/upload_files.rs index 71fcbc4..196f3cd 100644 --- a/tests/upload_files.rs +++ b/tests/upload_files.rs @@ -6,6 +6,8 @@ use reqwest::blocking::{multipart, Client}; use rstest::rstest; use select::document::Document; use select::predicate::{Attr, Text}; +use std::fs::create_dir_all; +use std::path::Path; #[rstest] fn uploading_files_works(#[with(&["-u"])] server: TestServer) -> Result<(), Error> { @@ -72,7 +74,7 @@ fn uploading_files_is_prevented(server: TestServer) -> Result<(), Error> { .error_for_status() .is_err()); - // After uploading, check whether the uploaded file is now getting listed. + // After uploading, check whether the uploaded file is NOT getting listed. let body = reqwest::blocking::get(server.url())?; let parsed = Document::from_read(body)?; assert!(!parsed.find(Text).any(|x| x.text() == test_file_name)); @@ -80,6 +82,92 @@ fn uploading_files_is_prevented(server: TestServer) -> Result<(), Error> { Ok(()) } +/// This test runs the server with --allowed-upload-dir argument and +/// checks that file upload to a different directory is actually prevented. +#[rstest] +#[case(server_no_stderr(&["-u", "someDir"]))] +#[case(server_no_stderr(&["-u", "someDir/some_sub_dir"]))] +fn uploading_files_is_restricted(#[case] server: TestServer) -> Result<(), Error> { + let test_file_name = "uploaded test file.txt"; + + // Then try to upload file to root directory (which is not the --allowed-upload-dir) + let form = multipart::Form::new(); + let part = multipart::Part::text("this should not be uploaded") + .file_name(test_file_name) + .mime_str("text/plain")?; + let form = form.part("file_to_upload", part); + + let client = Client::new(); + // Ensure uploading fails and returns an error + assert_eq!( + 403, + client + .post(server.url().join("/upload?path=/")?) + .multipart(form) + .send()? + .status() + ); + + // After uploading, check whether the uploaded file is NOT getting listed. + let body = reqwest::blocking::get(server.url())?; + let parsed = Document::from_read(body)?; + assert!(!parsed.find(Text).any(|x| x.text() == test_file_name)); + + Ok(()) +} + +/// This tests that we can upload files to the directory specified by --allow-upload-dir +#[rstest] +#[case(server(&["-u", "someDir"]), vec!["someDir"])] +#[case(server(&["-u", "./-someDir"]), vec!["./-someDir"])] +#[case(server(&["-u", Path::new("someDir/some_sub_dir").to_str().unwrap()]), + vec!["someDir/some_sub_dir"])] +#[case(server(&["-u", Path::new("someDir/some_sub_dir").to_str().unwrap(), + "-u", Path::new("someDir/some_other_dir").to_str().unwrap()]), + vec!["someDir/some_sub_dir", "someDir/some_other_dir"])] +fn uploading_files_to_allowed_dir_works( + #[case] server: TestServer, + #[case] upload_dirs: Vec<&str>, +) -> Result<(), Error> { + let test_file_name = "uploaded test file.txt"; + + for upload_dir in upload_dirs { + // Create test directory + create_dir_all(server.path().join(Path::new(upload_dir))).unwrap(); + + // Before uploading, check whether the uploaded file does not yet exist. + let body = reqwest::blocking::get(server.url().join(upload_dir)?)?.error_for_status()?; + let parsed = Document::from_read(body)?; + assert!(parsed.find(Text).all(|x| x.text() != test_file_name)); + + // Perform the actual upload. + let upload_action = parsed + .find(Attr("id", "file_submit")) + .next() + .expect("Couldn't find element with id=file_submit") + .attr("action") + .expect("Upload form doesn't have action attribute"); + let form = multipart::Form::new(); + let part = multipart::Part::text("this should be uploaded") + .file_name(test_file_name) + .mime_str("text/plain")?; + let form = form.part("file_to_upload", part); + + let client = Client::new(); + client + .post(server.url().join(upload_action)?) + .multipart(form) + .send()? + .error_for_status()?; + + // After uploading, check whether the uploaded file is now getting listed. + let body = reqwest::blocking::get(server.url().join(upload_dir)?)?; + let parsed = Document::from_read(body)?; + assert!(parsed.find(Text).any(|x| x.text() == test_file_name)); + } + Ok(()) +} + /// Test for path traversal vulnerability (CWE-22) in both path parameter of query string and in /// file name (Content-Disposition) /// @@ -98,7 +186,6 @@ fn prevent_path_traversal_attacks( #[case] expected: &str, ) -> Result<(), Error> { // Create test directories - use std::fs::create_dir_all; create_dir_all(server.path().join("foo")).unwrap(); if !cfg!(windows) { for dir in &["C:/foo/C:", r"C:\foo", r"\foo"] { |