Merge pull request #473 from DCjanus/master

fix: check directory traversal attack without file system visit

Author: yoshuawuyts

src/server/serve_dir.rs

@@ -4,6 +4,7 @@ use crate::{Body, Endpoint, Request, Response, Result, StatusCode};
 use async_std::fs::File;
 use async_std::io::BufReader;
 
+use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 
 type BoxFuture<'a, T> = std::pin::Pin<Box<dyn std::future::Future<Output = T> + 'a + Send>>;
@@ -22,43 +23,38 @@ impl ServeDir {
 impl<State> Endpoint<State> for ServeDir {
     fn call<'a>(&'a self, req: Request<State>) -> BoxFuture<'a, Result> {
         let path = req.uri().path();
-        let path = path.replacen(&self.prefix, "", 1);
+        let path = path.trim_start_matches(&self.prefix);
         let path = path.trim_start_matches('/');
-        let mut dir = self.dir.clone();
+        let mut file_path = self.dir.clone();
         for p in Path::new(path) {
-            dir.push(&p);
+            if p == OsStr::new(".") {
+                continue;
+            } else if p == OsStr::new("..") {
+                file_path.pop();
+            } else {
+                file_path.push(&p);
+            }
         }
-        log::info!("Requested file: {:?}", dir);
+
+        log::info!("Requested file: {:?}", file_path);
 
         Box::pin(async move {
-            let file = match async_std::fs::canonicalize(&dir).await {
-                Err(_) => {
-                    // This needs to return the same status code as the
-                    // unauthorized case below to ensure we don't leak
-                    // information of which files exist to adversaries.
-                    log::warn!("File not found: {:?}", dir);
-                    return Ok(Response::new(StatusCode::NotFound));
-                }
-                Ok(mut file_path) => {
-                    // Verify this is a sub-path of the original dir.
-                    let mut file_iter = (&mut file_path).iter();
-                    if !dir.iter().all(|lhs| Some(lhs) == file_iter.next()) {
-                        // This needs to return the same status code as the
-                        // 404 case above to ensure we don't leak
-                        // information about the local fs to adversaries.
-                        log::warn!("Unauthorized attempt to read: {:?}", file_path);
-                        return Ok(Response::new(StatusCode::NotFound));
-                    }
+            if !file_path.starts_with(&self.dir) {
+                log::warn!("Unauthorized attempt to read: {:?}", file_path);
+                return Ok(Response::new(StatusCode::Forbidden));
+            }
 
-                    // Open the file and send back the contents.
-                    match File::open(&file_path).await {
-                        Ok(file) => file,
-                        Err(_) => {
-                            log::warn!("Could not open {:?}", file_path);
-                            return Ok(Response::new(StatusCode::InternalServerError));
-                        }
+            let file = match File::open(&file_path).await {
+                Err(error) => {
+                    return if error.kind() == async_std::io::ErrorKind::NotFound {
+                        log::warn!("File not found: {:?}", file_path);
+                        Ok(Response::new(StatusCode::NotFound))
+                    } else {
+                        log::warn!("Could not open {:?}", file_path);
+                        Ok(Response::new(StatusCode::InternalServerError))
                     }
                 }
+                Ok(file) => file,
             };
 
             let len = match file.metadata().await {