From fb278de4b42a2f2494d25d31e679dbd6b7c93d47 Mon Sep 17 00:00:00 2001
From: Thomas Harmon <thomas.alan.harmon@gmail.com>
Date: Tue, 9 Mar 2021 14:33:22 -0500
Subject: [PATCH] Prevent directory traversal attack

---
 actix-files/CHANGES.md        |  4 ++++
 actix-files/src/files.rs      |  3 ++-
 actix-files/src/service.rs    | 12 ++++++++++++
 actix-files/tests/encoding.rs |  8 ++++++++
 4 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md
index 6e2a241ac..9d1f08c22 100644
--- a/actix-files/CHANGES.md
+++ b/actix-files/CHANGES.md
@@ -1,6 +1,10 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
+* Prevent directory traversal attack by checking if the request path is a
+  descendant of the `serve_from` directory. E.g., if the files are served from
+  `./test/`, and someone requests `..%5c/some-secret-file`, they will get a
+  Forbidden response.
 
 
 ## 0.6.0-beta.2 - 2021-02-10
diff --git a/actix-files/src/files.rs b/actix-files/src/files.rs
index 6f8b28bbf..f5e9918ed 100644
--- a/actix-files/src/files.rs
+++ b/actix-files/src/files.rs
@@ -73,7 +73,8 @@ impl Files {
     ///
     /// The second argument (`serve_from`) is the location on disk at which files are loaded.
     /// This can be a relative path. For example, `./` would serve files from the current
-    /// working directory.
+    /// working directory. Requests for files outside of the `serve_from` directory
+    /// will receive a Forbidden Response.
     ///
     /// # Implementation Notes
     /// If the mount path is set as the root path `/`, services registered after this one will
diff --git a/actix-files/src/service.rs b/actix-files/src/service.rs
index 14eea6ebc..74a2402f6 100644
--- a/actix-files/src/service.rs
+++ b/actix-files/src/service.rs
@@ -88,6 +88,18 @@ impl Service<ServiceRequest> for FilesService {
             Err(e) => return self.handle_err(e, req),
         };
 
+        let is_path_descendant = path
+            .as_path()
+            .ancestors()
+            .any(|ancestor| ancestor == self.directory.as_path());
+        if !is_path_descendant {
+            return Either::Left(ok(req.into_response(
+                actix_web::HttpResponse::Forbidden()
+                    .insert_header(header::ContentType(mime::TEXT_PLAIN_UTF_8))
+                    .body("Requested files outside the server."),
+            )));
+        }
+
         if path.is_dir() {
             if let Some(ref redir_index) = self.index {
                 if self.redirect_to_slash && !req.path().ends_with('/') {
diff --git a/actix-files/tests/encoding.rs b/actix-files/tests/encoding.rs
index d21d4f8fd..941984c7b 100644
--- a/actix-files/tests/encoding.rs
+++ b/actix-files/tests/encoding.rs
@@ -35,4 +35,12 @@ async fn test_utf8_file_contents() {
         res.headers().get(header::CONTENT_TYPE),
         Some(&HeaderValue::from_static("text/plain; charset=utf-8")),
     );
+
+    // prevent directory traversal attack
+    let srv = test::init_service(App::new().service(Files::new("/", "./tests"))).await;
+
+    let req = TestRequest::with_uri("..%5c/README.md").to_request();
+    let res = test::call_service(&srv, req).await;
+
+    assert_eq!(res.status(), StatusCode::FORBIDDEN);
 }