From e6fcea67f456adc6765bbab32f0048ff2235d4c1 Mon Sep 17 00:00:00 2001
From: Thomas Harmon <thomas.alan.harmon@gmail.com>
Date: Tue, 9 Mar 2021 11:14:34 -0500
Subject: [PATCH] fix bug where URIs could traverse server filesystem

---
 actix-files/CHANGES.md      |  2 ++
 actix-files/src/path_buf.rs | 21 ++++++++++++++++-----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/actix-files/CHANGES.md b/actix-files/CHANGES.md
index 6e2a241ac..439bf59ad 100644
--- a/actix-files/CHANGES.md
+++ b/actix-files/CHANGES.md
@@ -1,6 +1,8 @@
 # Changes
 
 ## Unreleased - 2021-xx-xx
+* Fix `PathBufWrap::parse_path` to error if the `path` has `".."`. This is to
+  prevent directory traversal attacks.
 
 
 ## 0.6.0-beta.2 - 2021-02-10
diff --git a/actix-files/src/path_buf.rs b/actix-files/src/path_buf.rs
index dd8e5b503..e6463f68c 100644
--- a/actix-files/src/path_buf.rs
+++ b/actix-files/src/path_buf.rs
@@ -21,13 +21,13 @@ impl FromStr for PathBufWrap {
 
 impl PathBufWrap {
     /// Parse a path, giving the choice of allowing hidden files to be considered valid segments.
+    /// If the path contains `".."`, an error will be returned to prevent someone being able
+    /// to traverse the filesystem.
     pub fn parse_path(path: &str, hidden_files: bool) -> Result<Self, UriSegmentError> {
         let mut buf = PathBuf::new();
 
         for segment in path.split('/') {
-            if segment == ".." {
-                buf.pop();
-            } else if !hidden_files && segment.starts_with('.') {
+            if !hidden_files && segment.starts_with('.') {
                 return Err(UriSegmentError::BadStart('.'));
             } else if segment.starts_with('*') {
                 return Err(UriSegmentError::BadStart('*'));
@@ -41,6 +41,8 @@ impl PathBufWrap {
                 continue;
             } else if cfg!(windows) && segment.contains('\\') {
                 return Err(UriSegmentError::BadChar('\\'));
+            } else if segment.contains("..") {
+                return Err(UriSegmentError::BadStart('.'));
             } else {
                 buf.push(segment)
             }
@@ -99,8 +101,12 @@ mod tests {
             PathBuf::from_iter(vec!["seg1", "seg2"])
         );
         assert_eq!(
-            PathBufWrap::from_str("/seg1/../seg2/").unwrap().0,
-            PathBuf::from_iter(vec!["seg2"])
+            PathBufWrap::from_str("/seg1/../seg2/").map(|t| t.0),
+            Err(UriSegmentError::BadStart('.'))
+        );
+        assert_eq!(
+            PathBufWrap::from_str("/seg1/..%5c/..%5c/seg2/test.txt").map(|t| t.0),
+            Err(UriSegmentError::BadStart('.'))
         );
     }
 
@@ -115,5 +121,10 @@ mod tests {
             PathBufWrap::parse_path("/test/.tt", true).unwrap().0,
             PathBuf::from_iter(vec!["test", ".tt"])
         );
+
+        assert_eq!(
+            PathBufWrap::parse_path("/seg1/..%5c/..%5c/seg2/test.txt", true).map(|t| t.0),
+            Err(UriSegmentError::BadStart('.'))
+        );
     }
 }