fix(axum-extra): escape multipart Content-Disposition params (#3776)

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
 

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")

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"));
+    }
 }

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)
                 ),
             );
         }

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")]

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<u8> {
+    pub(super) fn serialize(&self) -> Result<Vec<u8>, &'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<dyn std::error::Error>> {
+        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 {