From b90b8e02d0f761ce36a13610acd2afa60984a5e2 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Mon, 29 Jun 2026 11:26:47 +0200 Subject: [PATCH] fix(axum-extra): escape multipart Content-Disposition params (#3776) --- axum-extra/CHANGELOG.md | 3 + axum-extra/src/response/attachment.rs | 7 +- .../src/response/content_disposition.rs | 22 ++++-- axum-extra/src/response/file_stream.rs | 2 +- axum-extra/src/response/mod.rs | 2 +- axum-extra/src/response/multiple.rs | 69 +++++++++++++++++-- 6 files changed, 90 insertions(+), 15 deletions(-) diff --git a/axum-extra/CHANGELOG.md b/axum-extra/CHANGELOG.md index 675304a9..95820146 100644 --- a/axum-extra/CHANGELOG.md +++ b/axum-extra/CHANGELOG.md @@ -22,9 +22,12 @@ and this project adheres to [Semantic Versioning]. .route_with_tsr("/path", get(/* handler */)) .route_with_tsr("/path", post(/* handler */)) ``` +- **fixed:** Escape multipart `Content-Disposition` parameters and reject + newlines in field names and filenames ([#3776]) [#3599]: https://github.com/tokio-rs/axum/pull/3599 [#3586]: https://github.com/tokio-rs/axum/pull/3586 +[#3776]: https://github.com/tokio-rs/axum/pull/3776 # 0.12.6 diff --git a/axum-extra/src/response/attachment.rs b/axum-extra/src/response/attachment.rs index 87a8680c..c1246f59 100644 --- a/axum-extra/src/response/attachment.rs +++ b/axum-extra/src/response/attachment.rs @@ -1,4 +1,4 @@ -use super::content_disposition::EscapedFilename; +use super::content_disposition::EscapedQuotedString; use axum_core::response::IntoResponse; use http::{header, HeaderMap, HeaderValue}; use tracing::error; @@ -91,7 +91,10 @@ where let filename_str = filename .to_str() .expect("This was a HeaderValue so this can not fail"); - let value = format!("attachment; filename=\"{}\"", EscapedFilename(filename_str)); + let value = format!( + "attachment; filename=\"{}\"", + EscapedQuotedString(filename_str) + ); HeaderValue::try_from(value).expect("This was a HeaderValue so this can not fail") } else { HeaderValue::from_static("attachment") diff --git a/axum-extra/src/response/content_disposition.rs b/axum-extra/src/response/content_disposition.rs index a4a34884..32fa224f 100644 --- a/axum-extra/src/response/content_disposition.rs +++ b/axum-extra/src/response/content_disposition.rs @@ -5,9 +5,9 @@ use std::fmt::{self, Write}; /// /// This prevents Content-Disposition header parameter injection /// (similar to CVE-2023-29401). -pub(crate) struct EscapedFilename<'a>(pub &'a str); +pub(crate) struct EscapedQuotedString<'a>(pub &'a str); -impl fmt::Display for EscapedFilename<'_> { +impl fmt::Display for EscapedQuotedString<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { for c in self.0.chars() { if c == '\\' || c == '"' { @@ -19,19 +19,24 @@ impl fmt::Display for EscapedFilename<'_> { } } +#[cfg(any(feature = "multipart", test))] +pub(crate) fn contains_newlines(value: &str) -> bool { + value.contains(['\r', '\n']) +} + #[cfg(test)] mod tests { use super::*; #[test] fn no_special_characters() { - assert_eq!(EscapedFilename("report.pdf").to_string(), "report.pdf"); + assert_eq!(EscapedQuotedString("report.pdf").to_string(), "report.pdf"); } #[test] fn escapes_double_quotes() { assert_eq!( - EscapedFilename("evil\"; filename*=UTF-8''pwned.txt; x=\"").to_string(), + EscapedQuotedString("evil\"; filename*=UTF-8''pwned.txt; x=\"").to_string(), "evil\\\"; filename*=UTF-8''pwned.txt; x=\\\"", ); } @@ -39,8 +44,15 @@ mod tests { #[test] fn escapes_backslashes() { assert_eq!( - EscapedFilename("file\\name.txt").to_string(), + EscapedQuotedString("file\\name.txt").to_string(), "file\\\\name.txt", ); } + + #[test] + fn detects_newlines() { + assert!(contains_newlines("line\r\nbreak")); + assert!(contains_newlines("line\nbreak")); + assert!(!contains_newlines("report.pdf")); + } } diff --git a/axum-extra/src/response/file_stream.rs b/axum-extra/src/response/file_stream.rs index 95f22476..3ea66e9c 100644 --- a/axum-extra/src/response/file_stream.rs +++ b/axum-extra/src/response/file_stream.rs @@ -278,7 +278,7 @@ where header::CONTENT_DISPOSITION, format!( "attachment; filename=\"{}\"", - super::content_disposition::EscapedFilename(&file_name) + super::content_disposition::EscapedQuotedString(&file_name) ), ); } diff --git a/axum-extra/src/response/mod.rs b/axum-extra/src/response/mod.rs index e3ec32fc..65a010f2 100644 --- a/axum-extra/src/response/mod.rs +++ b/axum-extra/src/response/mod.rs @@ -1,6 +1,6 @@ //! Additional types for generating responses. -#[cfg(any(feature = "attachment", feature = "file-stream"))] +#[cfg(any(feature = "attachment", feature = "file-stream", feature = "multipart"))] mod content_disposition; #[cfg(feature = "erased-json")] diff --git a/axum-extra/src/response/multiple.rs b/axum-extra/src/response/multiple.rs index 1d5f0cb5..47eb386d 100644 --- a/axum-extra/src/response/multiple.rs +++ b/axum-extra/src/response/multiple.rs @@ -1,5 +1,6 @@ //! Generate forms to use in responses. +use super::content_disposition::{contains_newlines, EscapedQuotedString}; use axum_core::response::{IntoResponse, Response}; use fastrand; use http::{header, HeaderMap, StatusCode}; @@ -53,7 +54,14 @@ impl IntoResponse for MultipartForm { for part in self.parts { // for each part, the boundary is preceded by two dashes serialized_form.extend_from_slice(format!("--{boundary}\r\n").as_bytes()); - serialized_form.extend_from_slice(&part.serialize()); + let Ok(serialized_part) = part.serialize() else { + return ( + StatusCode::INTERNAL_SERVER_ERROR, + "Invalid multipart field name or filename", + ) + .into_response(); + }; + serialized_form.extend_from_slice(&serialized_part); } serialized_form.extend_from_slice(format!("--{boundary}--").as_bytes()); (headers, serialized_form).into_response() @@ -173,7 +181,7 @@ impl Part { } /// Serialize this part into a chunk that can be easily inserted into a larger form - pub(super) fn serialize(&self) -> Vec { + pub(super) fn serialize(&self) -> Result, &'static str> { // A part is serialized in this general format: // // the filename is optional // Content-Disposition: form-data; name="FIELD_NAME"; filename="FILENAME"\r\n @@ -183,11 +191,21 @@ impl Part { // \r\n // CONTENTS\r\n + if contains_newlines(&self.name) { + return Err("Invalid multipart field name"); + } + // Format what we can as a string, then handle the rest at a byte level - let mut serialized_part = format!("Content-Disposition: form-data; name=\"{}\"", self.name); + let mut serialized_part = format!( + "Content-Disposition: form-data; name=\"{}\"", + EscapedQuotedString(&self.name) + ); // specify a filename if one was set if let Some(filename) = &self.filename { - serialized_part += &format!("; filename=\"{filename}\""); + if contains_newlines(filename) { + return Err("Invalid multipart filename"); + } + serialized_part += &format!("; filename=\"{}\"", EscapedQuotedString(filename)); } serialized_part += "\r\n"; // specify the MIME type @@ -197,7 +215,7 @@ impl Part { part_bytes.extend_from_slice(&self.contents); part_bytes.extend_from_slice(b"\r\n"); - part_bytes + Ok(part_bytes) } } @@ -227,7 +245,7 @@ mod tests { use super::{generate_boundary, MultipartForm, Part}; use axum::{body::Body, http}; use axum::{routing::get, Router}; - use http::{Request, Response}; + use http::{Request, Response, StatusCode}; use http_body_util::BodyExt; use mime::Mime; use tower::ServiceExt; @@ -282,6 +300,45 @@ mod tests { Ok(()) } + #[test] + fn multipart_part_escapes_content_disposition_params() { + let part = Part::file( + "field\"; injected=\"1", + "evil\\name\"; filename*=UTF-8''pwned.txt; x=\"", + b"hi".to_vec(), + ); + + let body = String::from_utf8(part.serialize().unwrap()).unwrap(); + + assert!(body.starts_with( + "Content-Disposition: form-data; name=\"field\\\"; injected=\\\"1\"; filename=\"evil\\\\name\\\"; filename*=UTF-8''pwned.txt; x=\\\"\"\r\n" + )); + } + + #[tokio::test] + async fn multipart_form_rejects_newlines_in_field_metadata( + ) -> Result<(), Box> { + async fn handle() -> MultipartForm { + MultipartForm::from_iter(vec![Part::file( + "bad\r\nx-extra: injected", + "file.txt", + b"hi".to_vec(), + )]) + } + + let app = Router::new().route("/", get(handle)); + let response = app + .oneshot(Request::builder().uri("/").body(Body::empty())?) + .await?; + + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); + + let body = response.into_body().collect().await?.to_bytes(); + assert_eq!(&body[..], b"Invalid multipart field name or filename"); + + Ok(()) + } + #[test] fn valid_boundary_generation() { for _ in 0..256 {