fix(axum-extra): escape filename in Content-Disposition header (#3664)

axum-extra/CHANGELOG.md

@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog],
 and this project adheres to [Semantic Versioning].
 
+# 0.12.6
+
+- **fixed:** Escape backslashes and double quotes in `Content-Disposition` filenames
+  to prevent header parameter injection in `Attachment` and `FileStream` ([#3664])
+
+[#3664]: https://github.com/tokio-rs/axum/pull/3664
+
 # 0.12.5
 
 - **fixed:** `JsonLines` now correctly respects the default body limit ([#3591])

axum-extra/src/response/attachment.rs

@@ -1,3 +1,4 @@
+use super::content_disposition::EscapedFilename;
 use axum_core::response::IntoResponse;
 use http::{header, HeaderMap, HeaderValue};
 use tracing::error;
@@ -87,11 +88,11 @@ where
         }
 
         let content_disposition = if let Some(filename) = self.filename {
-            let mut bytes = b"attachment; filename=\"".to_vec();
+            let filename_str = filename
-            bytes.extend_from_slice(filename.as_bytes());
+                .to_str()
-            bytes.push(b'\"');
+                .expect("This was a HeaderValue so this can not fail");
-
+            let value = format!("attachment; filename=\"{}\"", EscapedFilename(filename_str));
-            HeaderValue::from_bytes(&bytes).expect("This was a HeaderValue so this can not fail")
+            HeaderValue::try_from(value).expect("This was a HeaderValue so this can not fail")
         } else {
             HeaderValue::from_static("attachment")
         };
@@ -101,3 +102,49 @@ where
         (headers, self.inner).into_response()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use axum_core::response::IntoResponse;
+    use http::header::CONTENT_DISPOSITION;
+
+    #[test]
+    fn attachment_without_filename() {
+        let attachment = Attachment::new("data").into_response();
+        let value = attachment.headers().get(CONTENT_DISPOSITION).unwrap();
+        assert_eq!(value, "attachment");
+    }
+
+    #[test]
+    fn attachment_with_normal_filename() {
+        let attachment = Attachment::new("data")
+            .filename("report.pdf")
+            .into_response();
+        let value = attachment.headers().get(CONTENT_DISPOSITION).unwrap();
+        assert_eq!(value, "attachment; filename=\"report.pdf\"");
+    }
+
+    #[test]
+    fn attachment_filename_escapes_quotes() {
+        // A filename containing a double quote should be escaped to prevent
+        // Content-Disposition parameter injection (see CVE-2023-29401)
+        let attachment = Attachment::new("data")
+            .filename("evil\"; filename*=UTF-8''pwned.txt; x=\"")
+            .into_response();
+        let value = attachment.headers().get(CONTENT_DISPOSITION).unwrap();
+        assert_eq!(
+            value,
+            "attachment; filename=\"evil\\\"; filename*=UTF-8''pwned.txt; x=\\\"\""
+        );
+    }
+
+    #[test]
+    fn attachment_filename_escapes_backslashes() {
+        let attachment = Attachment::new("data")
+            .filename("file\\name.txt")
+            .into_response();
+        let value = attachment.headers().get(CONTENT_DISPOSITION).unwrap();
+        assert_eq!(value, "attachment; filename=\"file\\\\name.txt\"");
+    }
+}

axum-extra/src/response/content_disposition.rs

@@ -0,0 +1,46 @@
+use std::fmt::{self, Write};
+
+/// A wrapper type that escapes backslashes and double quotes when formatted,
+/// for safe inclusion in Content-Disposition header quoted-strings.
+///
+/// This prevents Content-Disposition header parameter injection
+/// (similar to CVE-2023-29401).
+pub(crate) struct EscapedFilename<'a>(pub &'a str);
+
+impl fmt::Display for EscapedFilename<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for c in self.0.chars() {
+            if c == '\\' || c == '"' {
+                f.write_char('\\')?;
+            }
+            f.write_char(c)?;
+        }
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn no_special_characters() {
+        assert_eq!(EscapedFilename("report.pdf").to_string(), "report.pdf");
+    }
+
+    #[test]
+    fn escapes_double_quotes() {
+        assert_eq!(
+            EscapedFilename("evil\"; filename*=UTF-8''pwned.txt; x=\"").to_string(),
+            "evil\\\"; filename*=UTF-8''pwned.txt; x=\\\"",
+        );
+    }
+
+    #[test]
+    fn escapes_backslashes() {
+        assert_eq!(
+            EscapedFilename("file\\name.txt").to_string(),
+            "file\\\\name.txt",
+        );
+    }
+}

axum-extra/src/response/file_stream.rs

@@ -276,7 +276,10 @@ where
         if let Some(file_name) = self.file_name {
             resp = resp.header(
                 header::CONTENT_DISPOSITION,
-                format!("attachment; filename=\"{file_name}\""),
+                format!(
+                    "attachment; filename=\"{}\"",
+                    super::content_disposition::EscapedFilename(&file_name)
+                ),
             );
         }
 
@@ -601,6 +604,59 @@ mod tests {
         Some((start, end))
     }
 
+    #[tokio::test]
+    async fn filename_escapes_quotes() -> Result<(), Box<dyn std::error::Error>> {
+        let app = Router::new().route(
+            "/file",
+            get(|| async {
+                let file_content = b"data".to_vec();
+                let reader = Cursor::new(file_content);
+                let stream = ReaderStream::new(reader);
+                // Filename containing double quotes that could cause parameter injection
+                FileStream::new(stream)
+                    .file_name("evil\"; filename*=UTF-8''pwned.txt; x=\"")
+                    .into_response()
+            }),
+        );
+
+        let response = app
+            .oneshot(Request::builder().uri("/file").body(Body::empty())?)
+            .await?;
+
+        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(
+            response.headers().get("content-disposition").unwrap(),
+            "attachment; filename=\"evil\\\"; filename*=UTF-8''pwned.txt; x=\\\"\""
+        );
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn filename_escapes_backslashes() -> Result<(), Box<dyn std::error::Error>> {
+        let app = Router::new().route(
+            "/file",
+            get(|| async {
+                let file_content = b"data".to_vec();
+                let reader = Cursor::new(file_content);
+                let stream = ReaderStream::new(reader);
+                FileStream::new(stream)
+                    .file_name("file\\name.txt")
+                    .into_response()
+            }),
+        );
+
+        let response = app
+            .oneshot(Request::builder().uri("/file").body(Body::empty())?)
+            .await?;
+
+        assert_eq!(response.status(), StatusCode::OK);
+        assert_eq!(
+            response.headers().get("content-disposition").unwrap(),
+            "attachment; filename=\"file\\\\name.txt\""
+        );
+        Ok(())
+    }
+
     #[tokio::test]
     async fn response_range_empty_file() -> Result<(), Box<dyn std::error::Error>> {
         let file = tempfile::NamedTempFile::new()?;

axum-extra/src/response/mod.rs

@@ -1,5 +1,8 @@
 //! Additional types for generating responses.
 
+#[cfg(any(feature = "attachment", feature = "file-stream"))]
+mod content_disposition;
+
 #[cfg(feature = "erased-json")]
 mod erased_json;