From be71e7b286829b61a3e2451505378125c05c11c0 Mon Sep 17 00:00:00 2001 From: David Pedersen Date: Tue, 17 May 2022 11:38:15 +0100 Subject: [PATCH] Fix vulnerability in example-stream-to-file example (#1040) * Fix vulnerability in example-stream-to-file example * Save files to separate directory --- examples/stream-to-file/src/main.rs | 59 +++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/examples/stream-to-file/src/main.rs b/examples/stream-to-file/src/main.rs index 407d379d..e9b816b1 100644 --- a/examples/stream-to-file/src/main.rs +++ b/examples/stream-to-file/src/main.rs @@ -18,6 +18,8 @@ use tokio::{fs::File, io::BufWriter}; use tokio_util::io::StreamReader; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; +const UPLOADS_DIRECTORY: &str = "uploads"; + #[tokio::main] async fn main() { tracing_subscriber::registry() @@ -27,6 +29,11 @@ async fn main() { .with(tracing_subscriber::fmt::layer()) .init(); + // save files to a separte directory to not override files in the current directory + tokio::fs::create_dir(UPLOADS_DIRECTORY) + .await + .expect("failed to create `uploads` directory"); + let app = Router::new() .route("/", get(show_form).post(accept_form)) .route("/file/:file_name", post(save_request_body)); @@ -46,9 +53,7 @@ async fn save_request_body( Path(file_name): Path, body: BodyStream, ) -> Result<(), (StatusCode, String)> { - stream_to_file(&file_name, body) - .await - .map_err(|err| (StatusCode::INTERNAL_SERVER_ERROR, err.to_string())) + stream_to_file(&file_name, body).await } // Handler that returns HTML for a multipart form. @@ -88,30 +93,52 @@ async fn accept_form(mut multipart: Multipart) -> Result(path: &str, stream: S) -> Result<(), io::Error> +async fn stream_to_file(path: &str, stream: S) -> Result<(), (StatusCode, String)> where S: Stream>, E: Into, { - // Convert the stream into an `AsyncRead`. - let body_with_io_error = stream.map_err(|err| io::Error::new(io::ErrorKind::Other, err)); - let body_reader = StreamReader::new(body_with_io_error); - futures::pin_mut!(body_reader); + if !path_is_valid(path) { + return Err((StatusCode::BAD_REQUEST, "Invalid path".to_owned())); + } - // Create the file. `File` implements `AsyncWrite`. - let mut file = BufWriter::new(File::create(path).await?); + async { + // Convert the stream into an `AsyncRead`. + let body_with_io_error = stream.map_err(|err| io::Error::new(io::ErrorKind::Other, err)); + let body_reader = StreamReader::new(body_with_io_error); + futures::pin_mut!(body_reader); - // Copy the body into the file. - tokio::io::copy(&mut body_reader, &mut file).await?; + // Create the file. `File` implements `AsyncWrite`. + let path = std::path::Path::new(UPLOADS_DIRECTORY).join(path); + let mut file = BufWriter::new(File::create(path).await?); - Ok(()) + // Copy the body into the file. + tokio::io::copy(&mut body_reader, &mut file).await?; + + Ok::<_, io::Error>(()) + } + .await + .map_err(|err| (StatusCode::INTERNAL_SERVER_ERROR, err.to_string())) +} + +// to prevent directory traversal attacks we ensure the path conists of exactly one normal +// component +fn path_is_valid(path: &str) -> bool { + let path = std::path::Path::new(&*path); + let mut components = path.components().peekable(); + + if let Some(first) = components.peek() { + if !matches!(first, std::path::Component::Normal(_)) { + return false; + } + } + + components.count() == 1 }