In ServeDir, change the behaviour for handling symlinks

tower-http/Cargo.toml

@@ -39,6 +39,10 @@ tracing = { version = "0.1", default-features = false, optional = true }
 httpdate = { version = "1.0", optional = true }
 uuid = { version = "1.0", features = ["v4"], optional = true }
 
+# linux only dependencies
+[target.'cfg(target_os = "linux")'.dependencies]
+rustix = { version = "1", features = ["fs"] }
+
 [dev-dependencies]
 brotli = "8"
 bytes = "1"
@@ -55,6 +59,7 @@ tokio = { version = "1", features = ["full"] }
 tower = { version = "0.5", features = ["buffer", "util", "retry", "make", "timeout"] }
 tracing-subscriber = "0.3"
 zstd = "0.13"
+tempfile = "3"
 
 [features]
 default = []

tower-http/src/services/fs/serve_dir/mod.rs

@@ -40,6 +40,10 @@ const DEFAULT_CAPACITY: usize = 65536;
 ///   existing file (`/file.html/something`)
 /// - We don't have necessary permissions to read the file
 ///
+/// On linux, if a file is a symlink to a file outside the base directory,
+/// a 500 error will be returned, unless `follow_symlinks_outside_base`
+/// is set to true.
+///
 /// # Example
 ///
 /// ```
@@ -59,6 +63,7 @@ pub struct ServeDir<F = DefaultServeDirFallback> {
     variant: ServeVariant,
     fallback: Option<F>,
     call_fallback_on_method_not_allowed: bool,
+    follow_symlinks_outside_base: bool,
 }
 
 impl ServeDir<DefaultServeDirFallback> {
@@ -79,6 +84,7 @@ impl ServeDir<DefaultServeDirFallback> {
             },
             fallback: None,
             call_fallback_on_method_not_allowed: false,
+            follow_symlinks_outside_base: false,
         }
     }
 
@@ -93,6 +99,7 @@ impl ServeDir<DefaultServeDirFallback> {
             variant: ServeVariant::SingleFile { mime },
             fallback: None,
             call_fallback_on_method_not_allowed: false,
+            follow_symlinks_outside_base: false,
         }
     }
 }
@@ -217,6 +224,7 @@ impl<F> ServeDir<F> {
             variant: self.variant,
             fallback: Some(new_fallback),
             call_fallback_on_method_not_allowed: self.call_fallback_on_method_not_allowed,
+            follow_symlinks_outside_base: self.follow_symlinks_outside_base,
         }
     }
 
@@ -249,6 +257,16 @@ impl<F> ServeDir<F> {
         self
     }
 
+    /// Start following symlinks outside the base directory.
+    ///
+    /// Warning: if you have a writeable directory inside the base directory, this
+    /// means that a local user can exfiltrate any file that the server process have
+    /// read access to.
+    pub fn follow_symlinks_outside_base(mut self, follow_symlinks_outside_base: bool) -> Self {
+        self.follow_symlinks_outside_base = follow_symlinks_outside_base;
+        self
+    }
+
     /// Call the service and get a future that contains any `std::io::Error` that might have
     /// happened.
     ///
@@ -381,6 +399,8 @@ impl<F> ServeDir<F> {
             negotiated_encodings,
             range_header,
             buf_chunk_size,
+            self.base.clone(),
+            self.follow_symlinks_outside_base,
         ));
 
         ResponseFuture::open_file_future(open_file_future, fallback_and_request)

tower-http/src/services/fs/serve_dir/open_file.rs

@@ -40,13 +40,16 @@ pub(super) enum FileRequestExtent {
     Head(Metadata),
 }
 
+#[allow(clippy::too_many_arguments)]
 pub(super) async fn open_file(
     variant: ServeVariant,
     mut path_to_file: PathBuf,
     req: Request<Empty<Bytes>>,
     negotiated_encodings: Vec<(Encoding, QValue)>,
     range_header: Option<String>,
     buf_chunk_size: usize,
+    base_path: PathBuf,
+    follow_symlinks_outside_base: bool,
 ) -> io::Result<OpenFileOutput> {
     let if_unmodified_since = req
         .headers()
@@ -110,15 +113,21 @@ pub(super) async fn open_file(
             last_modified,
         })))
     } else {
-        let (mut file, maybe_encoding) =
-            match open_file_with_fallback(path_to_file, negotiated_encodings).await {
-                Ok(result) => result,
+        let (mut file, maybe_encoding) = match open_file_with_fallback(
+            path_to_file,
+            negotiated_encodings,
+            &base_path,
+            follow_symlinks_outside_base,
+        )
+        .await
+        {
+            Ok(result) => result,
 
-                Err(err) if is_invalid_filename_error(&err) => {
-                    return Ok(OpenFileOutput::InvalidFilename)
-                }
-                Err(err) => return Err(err),
-            };
+            Err(err) if is_invalid_filename_error(&err) => {
+                return Ok(OpenFileOutput::InvalidFilename)
+            }
+            Err(err) => return Err(err),
+        };
 
         let meta = file.metadata().await?;
         let last_modified = meta.modified().ok().map(LastModified::from);
@@ -226,17 +235,74 @@ fn preferred_encoding(
     preferred_encoding
 }
 
+fn canonicalize_and_openat2(base_path: &Path, path: &Path) -> io::Result<File> {
+    let (path, base_path2) = if base_path.is_file() || !base_path.exists() {
+        let base_path = base_path.parent().unwrap().canonicalize()?;
+        let path = path
+            .canonicalize()?
+            .strip_prefix(&base_path)
+            .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?
+            .to_owned();
+        (
+            path,
+            rustix::fs::open(
+                base_path,
+                rustix::fs::OFlags::RDONLY,
+                rustix::fs::Mode::empty(),
+            )?,
+        )
+    } else {
+        let base_path = base_path.canonicalize()?;
+        let path = path
+            .canonicalize()?
+            .strip_prefix(&base_path)
+            .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?
+            .to_owned();
+        (
+            path,
+            rustix::fs::open(
+                base_path,
+                rustix::fs::OFlags::RDONLY,
+                rustix::fs::Mode::empty(),
+            )?,
+        )
+    };
+
+    rustix::fs::openat2(
+        base_path2,
+        &path,
+        rustix::fs::OFlags::RDONLY,
+        rustix::fs::Mode::empty(),
+        rustix::fs::ResolveFlags::BENEATH,
+    )
+    .map(std::fs::File::from)
+    .map(File::from)
+    .map_err(|err| io::Error::new(io::ErrorKind::Other, err))
+}
+
 // Attempts to open the file with any of the possible negotiated_encodings in the
 // preferred order. If none of the negotiated_encodings have a corresponding precompressed
 // file the uncompressed file is used as a fallback.
 async fn open_file_with_fallback(
     mut path: PathBuf,
     mut negotiated_encoding: Vec<(Encoding, QValue)>,
+    base_path: &Path,
+    follow_symlinks_outside_base: bool,
 ) -> io::Result<(File, Option<Encoding>)> {
     let (file, encoding) = loop {
         // Get the preferred encoding among the negotiated ones.
         let encoding = preferred_encoding(&mut path, &negotiated_encoding);
-        match (File::open(&path).await, encoding) {
+        let file = {
+            #[cfg(target_os = "linux")]
+            if follow_symlinks_outside_base {
+                File::open(&path).await
+            } else {
+                canonicalize_and_openat2(base_path, &path)
+            }
+            #[cfg(not(target_os = "linux"))]
+            File::open(&path).await
+        };
+        match (file, encoding) {
             (Ok(file), maybe_encoding) => break (file, maybe_encoding),
             (Err(err), Some(encoding)) if err.kind() == io::ErrorKind::NotFound => {
                 // Remove the extension corresponding to a precompressed file (.gz, .br, .zz)

tower-http/src/services/fs/serve_dir/tests.rs

@@ -10,7 +10,9 @@ use http_body::Body as HttpBody;
 use http_body_util::BodyExt;
 use std::convert::Infallible;
 use std::fs;
-use std::io::Read;
+use std::fs::{create_dir, File};
+use std::io::{Read, Write};
+use std::os::unix::fs::symlink;
 use tower::{service_fn, ServiceExt};
 
 #[tokio::test]
@@ -627,6 +629,23 @@ async fn read_partial_errs_on_bad_range() {
     )
 }
 
+#[tokio::test]
+async fn read_partial_errs_on_bad_range_follow_symlinks() {
+    let svc = ServeDir::new("..").follow_symlinks_outside_base(true);
+    let req = Request::builder()
+        .uri("/README.md")
+        .header("Range", "bytes=-1-15")
+        .body(Body::empty())
+        .unwrap();
+    let res = svc.oneshot(req).await.unwrap();
+    assert_eq!(res.status(), StatusCode::RANGE_NOT_SATISFIABLE);
+    let file_contents = std::fs::read("../README.md").unwrap();
+    assert_eq!(
+        res.headers()["content-range"],
+        &format!("bytes */{}", file_contents.len())
+    )
+}
+
 #[tokio::test]
 async fn accept_encoding_identity() {
     let svc = ServeDir::new("..");
@@ -877,3 +896,55 @@ async fn calls_fallback_on_null() {
 
     assert_eq!(res.headers()["from-fallback"], "1");
 }
+
+#[tokio::test]
+async fn returns_500_if_symlink_outside_base() {
+    let tmp = tempfile::tempdir().unwrap();
+    let base = tmp.path().join("base");
+    create_dir(&base).unwrap();
+    let file_outside = tmp.path().join("file_outside.txt");
+    let mut outside = File::create(&file_outside).unwrap();
+    write!(outside, "{}", "outside").unwrap();
+    let file_inside = base.join("inside.txt");
+    symlink(file_outside, file_inside).unwrap();
+
+    let svc = ServeDir::new(base);
+
+    let request = Request::builder()
+        .header("Accept-Encoding", "deflate")
+        .method(Method::GET)
+        .uri("/inside.txt")
+        .body(Body::empty())
+        .unwrap();
+    let res = svc.oneshot(request).await.unwrap();
+
+    assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR);
+    assert!(res.headers().get(header::CONTENT_TYPE).is_none());
+}
+
+#[tokio::test]
+async fn allow_symlink_outside_base() {
+    let tmp = tempfile::tempdir().unwrap();
+    let base = tmp.path().join("base");
+    create_dir(&base).unwrap();
+    let file_outside = tmp.path().join("file_outside.txt");
+    let mut outside = File::create(&file_outside).unwrap();
+    write!(outside, "{}", "outside").unwrap();
+    let file_inside = base.join("inside.txt");
+    symlink(file_outside, file_inside).unwrap();
+
+    let svc = ServeDir::new(base).follow_symlinks_outside_base(true);
+
+    let request = Request::builder()
+        .header("Accept-Encoding", "deflate")
+        .method(Method::GET)
+        .uri("/inside.txt")
+        .body(Body::empty())
+        .unwrap();
+    let res = svc.oneshot(request).await.unwrap();
+
+    assert_eq!(res.status(), StatusCode::OK);
+    assert!(res.headers().get(header::CONTENT_TYPE).is_some());
+    let body = res.into_body().collect().await.unwrap().to_bytes();
+    assert_eq!("outside", String::from_utf8_lossy(&body));
+}

tower-http/src/services/fs/serve_file.rs

@@ -152,7 +152,10 @@ mod tests {
     use http::{Request, StatusCode};
     use http_body_util::BodyExt;
     use mime::Mime;
+    use std::fs::{create_dir, File};
     use std::io::Read;
+    use std::io::Write;
+    use std::os::unix::fs::symlink;
     use std::str::FromStr;
     use tokio::io::AsyncReadExt;
     use tower::ServiceExt;
@@ -496,6 +499,31 @@ mod tests {
         assert!(res.headers().get(header::CONTENT_TYPE).is_none());
     }
 
+    #[tokio::test]
+    async fn returns_500_if_symlink_outside_base() {
+        let tmp = tempfile::tempdir().unwrap();
+        let base = tmp.path().join("base");
+        create_dir(&base).unwrap();
+        let file_outside = tmp.path().join("file_outside.txt");
+        let mut outside = File::create(&file_outside).unwrap();
+        write!(outside, "{}", "outside").unwrap();
+        let file_inside = base.join("inside.txt");
+        symlink(file_outside, file_inside).unwrap();
+
+        let svc = ServeFile::new(base);
+
+        let request = Request::builder()
+            .header("Accept-Encoding", "deflate")
+            .method(Method::GET)
+            .uri("/inside.txt")
+            .body(Body::empty())
+            .unwrap();
+        let res = svc.oneshot(request).await.unwrap();
+
+        assert_eq!(res.status(), StatusCode::INTERNAL_SERVER_ERROR);
+        assert!(res.headers().get(header::CONTENT_TYPE).is_none());
+    }
+
     #[tokio::test]
     async fn last_modified() {
         let svc = ServeFile::new("../README.md");