Repository: thecoshman/http (language: rust)

Issue:
PUT allows file PUTting behind symlinks even with symlinks turned off



Discussion / hints:

Fix patch:
diff --git a/Cargo.toml b/Cargo.toml
index 2949f06..67759b6 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -9,7 +9,7 @@ categories = ["network-programming", "web-programming::http-server"]
 license = "MIT"
 build = "build.rs"
 # Remember to also update in appveyor.yml
-version = "1.3.3"
+version = "1.4.0"
 # Remember to also update in http.md
 authors = ["thecoshman <thecoshman@gmail.com>",
            "nabijaczleweli <nabijaczleweli@gmail.com>"]
diff --git a/appveyor.yml b/appveyor.yml
index 6f47fa4..7709ccf 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -1,4 +1,4 @@
-version: 1.3.3-{build}
+version: 1.4.0-{build}
 
 skip_tags: false
 
@@ -22,21 +22,21 @@ build: off
 build_script:
   - git submodule update --init --recursive
   - cargo build --verbose --release
-  - cp target\release\http.exe http-v1.3.3.exe
-  - strip --strip-all --remove-section=.comment --remove-section=.note http-v1.3.3.exe
-  - makensis -DHTTP_VERSION=v1.3.3 "/X!include \"EnvVarUpdate.nsh\"" install.nsi
+  - cp target\release\http.exe http-v1.4.0.exe
+  - strip --strip-all --remove-section=.comment --remove-section=.note http-v1.4.0.exe
+  - makensis -DHTTP_VERSION=v1.4.0 "/X!include \"EnvVarUpdate.nsh\"" install.nsi
 
 test: off
 test_script:
   - cargo test --verbose --release
 
 artifacts:
-  - path: http-v1.3.3.exe
-  - path: http v1.3.3 installer.exe
+  - path: http-v1.4.0.exe
+  - path: http v1.4.0 installer.exe
 
 deploy:
   provider: GitHub
-  artifact: /http.*v1.3.3.*\.exe/
+  artifact: /http.*v1.4.0.*\.exe/
   auth_token:
     secure: ZTXvCrv9y01s7Hd60w8W7NaouPnPoaw9YJt9WhWQ2Pep8HLvCikt9Exjkz8SGP9P
   on:
diff --git a/http.md b/http.md
index 8450c32..27fac0b 100644
--- a/http.md
+++ b/http.md
@@ -60,8 +60,16 @@ pass parameters like what port to use.
 
     Don't follow symlinks when requesting file access.
 
-    If a symlink is requested and this is on it will be treated as if it didn't
-    exist.
+    If a symlink is requested and this flag is on it will be treated as if it
+    didn't exist.
+
+  -r --sandbox-symlinks
+
+    Restrict/sandbox where symlinks lead to only the direct descendants
+    of the hosted directory.
+
+    If a file outside the direct descendancy of the hosted reictory requested
+    and this flag is on it will be treated as if it didn't exist.
 
   -w --allow-write
 
@@ -179,6 +187,14 @@ pass parameters like what port to use.
     Example output change:
       Hosting "." on port 8000 TLS certificate from "$TEMP/http-P-Rust-http/tls/tls.p12"...
 
+  `http -r`
+
+    As in the first example, but restrict accessible paths
+    to direct descendants of the hosted directory.
+
+    Example output change:
+      127.0.0.1:47916 requested to GET nonexistant entity S:\Rust-target\doc\main.css
+
 ## AUTHOR
 
 Written by thecoshman &lt;<thecoshman@gmail.com>&gt;,
diff --git a/src/ops.rs b/src/ops.rs
index 010139b..3e96e7f 100644
--- a/src/ops.rs
+++ b/src/ops.rs
@@ -22,9 +22,10 @@ use std::process::{ExitStatus, Command, Child, Stdio};
 use rfsapi::{RawFsApiHeader, FilesetData, RawFileData};
 use iron::{headers, status, method, mime, IronResult, Listening, Response, TypeMap, Request, Handler, Iron};
 use self::super::util::{url_path, file_hash, is_symlink, encode_str, encode_file, file_length, hash_string, html_response, file_binary, client_mobile,
-                        percent_decode, file_icon_suffix, is_actually_file, response_encoding, detect_file_as_dir, encoding_extension, file_time_modified,
-                        get_raw_fs_metadata, human_readable_size, USER_AGENT, ERROR_HTML, INDEX_EXTENSIONS, MIN_ENCODING_GAIN, MAX_ENCODING_SIZE,
-                        MIN_ENCODING_SIZE, DIRECTORY_LISTING_HTML, MOBILE_DIRECTORY_LISTING_HTML, BLACKLISTED_ENCODING_EXTENSIONS};
+                        percent_decode, file_icon_suffix, is_actually_file, is_descendant_of, response_encoding, detect_file_as_dir, encoding_extension,
+                        file_time_modified, get_raw_fs_metadata, human_readable_size, is_nonexistant_descendant_of, USER_AGENT, ERROR_HTML, INDEX_EXTENSIONS,
+                        MIN_ENCODING_GAIN, MAX_ENCODING_SIZE, MIN_ENCODING_SIZE, DIRECTORY_LISTING_HTML, MOBILE_DIRECTORY_LISTING_HTML,
+                        BLACKLISTED_ENCODING_EXTENSIONS};
 
 
 macro_rules! log {
@@ -64,6 +65,7 @@ type CacheT<Cnt> = HashMap<([u8; 32], String), Cnt>;
 pub struct HttpHandler {
     pub hosted_directory: (String, PathBuf),
     pub follow_symlinks: bool,
+    pub sandbox_symlinks: bool,
     pub check_indices: bool,
     pub writes_temp_dir: Option<(String, PathBuf)>,
     pub encoded_temp_dir: Option<(String, PathBuf)>,
@@ -76,6 +78,7 @@ impl HttpHandler {
         HttpHandler {
             hosted_directory: opts.hosted_directory.clone(),
             follow_symlinks: opts.follow_symlinks,
+            sandbox_symlinks: opts.sandbox_symlinks,
             check_indices: opts.check_indices,
             writes_temp_dir: HttpHandler::temp_subdir(&opts.temp_directory, opts.allow_writes, "writes"),
             encoded_temp_dir: HttpHandler::temp_subdir(&opts.temp_directory, opts.encode_fs, "encoded"),
@@ -144,7 +147,8 @@ impl HttpHandler {
 
         if url_err {
             self.handle_invalid_url(req, "<p>Percent-encoding decoded to invalid UTF-8.</p>")
-        } else if !req_p.exists() || (symlink && !self.follow_symlinks) {
+        } else if !req_p.exists() || (symlink && !self.follow_symlinks) ||
+                  (symlink && self.follow_symlinks && self.sandbox_symlinks && !is_descendant_of(&req_p, &self.hosted_directory.1)) {
             self.handle_nonexistant(req, req_p)
         } else if file && raw_fs {
             self.handle_get_raw_fs_file(req, req_p)
@@ -461,7 +465,16 @@ impl HttpHandler {
                                             files: req_p.read_dir()
                                                 .expect("Failed to read requested directory")
                                                 .map(|p| p.expect("Failed to iterate over requested directory"))
-                                                .filter(|f| self.follow_symlinks || !is_symlink(f.path()))
+                                                .filter(|f| {
+                    let fp = f.path();
+                    let mut symlink = false;
+                    !((!self.follow_symlinks &&
+                       (|| {
+                        symlink = is_symlink(&fp);
+                        symlink
+                    })()) ||
+                      (self.follow_symlinks && self.sandbox_symlinks && symlink && !is_descendant_of(fp, &self.hosted_directory.1)))
+                })
                                                 .map(|f| {
                     let is_file = is_actually_file(&f.file_type().expect("Failed to get file type"));
                     if is_file {
@@ -486,7 +499,9 @@ impl HttpHandler {
             if let Some(e) = INDEX_EXTENSIONS.iter()
                 .find(|e| {
                     idx.set_extension(e);
-                    idx.exists()
+                    idx.exists() &&
+                    ((!self.follow_symlinks || !self.sandbox_symlinks) ||
+                     (self.follow_symlinks && self.sandbox_symlinks && is_descendant_of(&req_p, &self.hosted_directory.1)))
                 }) {
                 if req.url.path().pop() == Some("") {
                     let r = self.handle_get_file(req, idx);
@@ -548,7 +563,15 @@ impl HttpHandler {
         let list_s = req_p.read_dir()
             .expect("Failed to read requested directory")
             .map(|p| p.expect("Failed to iterate over requested directory"))
-            .filter(|f| self.follow_symlinks || !is_symlink(f.path()))
+            .filter(|f| {
+                let fp = f.path();
+                let mut symlink = false;
+                !((!self.follow_symlinks &&
+                   (|| {
+                    symlink = is_symlink(&fp);
+                    symlink
+                })()) || (self.follow_symlinks && self.sandbox_symlinks && symlink && !is_descendant_of(fp, &self.hosted_directory.1)))
+            })
             .sorted_by(|lhs, rhs| {
                 (is_actually_file(&lhs.file_type().expect("Failed to get file type")),
                  lhs.file_name().to_str().expect("Failed to get file name").to_lowercase())
@@ -625,7 +648,15 @@ impl HttpHandler {
         let list_s = req_p.read_dir()
             .expect("Failed to read requested directory")
             .map(|p| p.expect("Failed to iterate over requested directory"))
-            .filter(|f| self.follow_symlinks || !is_symlink(f.path()))
+            .filter(|f| {
+                let fp = f.path();
+                let mut symlink = false;
+                !((!self.follow_symlinks &&
+                   (|| {
+                    symlink = is_symlink(&fp);
+                    symlink
+                })()) || (self.follow_symlinks && self.sandbox_symlinks && symlink && !is_descendant_of(fp, &self.hosted_directory.1)))
+            })
             .sorted_by(|lhs, rhs| {
                 (is_actually_file(&lhs.file_type().expect("Failed to get file type")),
                  lhs.file_name().to_str().expect("Failed to get file name").to_lowercase())
@@ -691,7 +722,7 @@ impl HttpHandler {
             return self.handle_forbidden_method(req, "-w", "write requests");
         }
 
-        let (req_p, _, url_err) = self.parse_requested_path(req);
+        let (req_p, symlink, url_err) = self.parse_requested_path(req);
 
         if url_err {
             self.handle_invalid_url(req, "<p>Percent-encoding decoded to invalid UTF-8.</p>")
@@ -701,9 +732,13 @@ impl HttpHandler {
             self.handle_invalid_url(req, "<p>Attempted to use file as directory.</p>")
         } else if req.headers.has::<headers::ContentRange>() {
             self.handle_put_partial_content(req)
+        } else if (symlink && !self.follow_symlinks) ||
+                  (symlink && self.follow_symlinks && self.sandbox_symlinks && !is_nonexistant_descendant_of(&req_p, &self.hosted_directory.1)) {
+            self.create_temp_dir(&self.writes_temp_dir);
+            self.handle_put_file(req, req_p, false)
         } else {
             self.create_temp_dir(&self.writes_temp_dir);
-            self.handle_put_file(req, req_p)
+            self.handle_put_file(req, req_p, true)
         }
     }
 
@@ -753,11 +788,17 @@ impl HttpHandler {
                                                                 ""]))
     }
 
-    fn handle_put_file(&self, req: &mut Request, req_p: PathBuf) -> IronResult<Response> {
-        let existant = req_p.exists();
+    fn handle_put_file(&self, req: &mut Request, req_p: PathBuf, legal: bool) -> IronResult<Response> {
+        let existant = !legal || req_p.exists();
         log!("{green}{}{reset} {} {magenta}{}{reset}, size: {}B",
              req.remote_addr,
-             if existant { "replaced" } else { "created" },
+             if !legal {
+                 "tried to illegally create"
+             } else if existant {
+                 "replaced"
+             } else {
+                 "created"
+             },
              req_p.display(),
              *req.headers.get::<headers::ContentLength>().expect("No Content-Length header"));
 
@@ -766,13 +807,15 @@ impl HttpHandler {
 
         io::copy(&mut req.body, &mut File::create(&temp_file_p).expect("Failed to create temp file"))
             .expect("Failed to write requested data to requested file");
-        let _ = fs::create_dir_all(req_p.parent().expect("Failed to get requested file's parent directory"));
-        fs::copy(&temp_file_p, req_p).expect("Failed to copy temp file to requested file");
+        if legal {
+            let _ = fs::create_dir_all(req_p.parent().expect("Failed to get requested file's parent directory"));
+            fs::copy(&temp_file_p, req_p).expect("Failed to copy temp file to requested file");
+        }
 
-        Ok(Response::with((if existant {
-                               status::NoContent
-                           } else {
+        Ok(Response::with((if !legal || !existant {
                                status::Created
+                           } else {
+                               status::NoContent
                            },
                            Header(headers::Server(USER_AGENT.to_string())))))
     }
@@ -786,7 +829,8 @@ impl HttpHandler {
 
         if url_err {
             self.handle_invalid_url(req, "<p>Percent-encoding decoded to invalid UTF-8.</p>")
-        } else if !req_p.exists() || (symlink && !self.follow_symlinks) {
+        } else if !req_p.exists() || (symlink && !self.follow_symlinks) ||
+                  (symlink && self.follow_symlinks && self.sandbox_symlinks && !is_descendant_of(&req_p, &self.hosted_directory.1)) {
             self.handle_nonexistant(req, req_p)
         } else {
             self.handle_delete_path(req, req_p, symlink)
@@ -949,6 +993,7 @@ impl Clone for HttpHandler {
         HttpHandler {
             hosted_directory: self.hosted_directory.clone(),
             follow_symlinks: self.follow_symlinks,
+            sandbox_symlinks: self.sandbox_symlinks,
             check_indices: self.check_indices,
             writes_temp_dir: self.writes_temp_dir.clone(),
             encoded_temp_dir: self.encoded_temp_dir.clone(),
diff --git a/src/options.rs b/src/options.rs
index 1b4b5ab..d04f014 100644
--- a/src/options.rs
+++ b/src/options.rs
@@ -27,6 +27,12 @@ pub struct Options {
     pub port: Option<u16>,
     /// Whether to allow symlinks to be requested. Default: true
     pub follow_symlinks: bool,
+    /// Whether to disallow going out of the descendants of the hosted directory (via symlinks)
+    ///
+    /// Can only be true if `follow_symlinks` is true.
+    ///
+    /// Default: false
+    pub sandbox_symlinks: bool,
     /// The temp directory to write to before copying to hosted directory and to store encoded FS responses.
     /// Default: `"$TEMP/http-[FULL_PATH_TO_HOSTED_DIR]"`
     pub temp_directory: (String, PathBuf),
@@ -56,6 +62,8 @@ impl Options {
             .arg(Arg::from_usage("-t --temp-dir [temp] 'Temporary directory. Default: $TEMP'")
                 .validator(|s| Options::filesystem_dir_validator(s, "Temporary directory")))
             .arg(Arg::from_usage("-s --no-follow-symlinks 'Don't follow symlinks. Default: false'"))
+            .arg(Arg::from_usage("-r --sandbox-symlinks 'Restrict/sandbox where symlinks lead to only the direct descendants of the hosted directory. \
+                                  Default: false'"))
             .arg(Arg::from_usage("-w --allow-write 'Allow for write operations. Default: false'"))
             .arg(Arg::from_usage("-i --no-indices 'Always generate dir listings even if index files are available. Default: false'"))
             .arg(Arg::from_usage("-e --no-encode 'Do not encode filesystem files. Default: false'"))
@@ -66,10 +74,12 @@ impl Options {
 
         let dir = matches.value_of("DIR").unwrap_or(".");
         let dir_pb = fs::canonicalize(dir).unwrap();
+        let follow_symlinks = !matches.is_present("no-follow-symlinks");
         Options {
             hosted_directory: (dir.to_string(), dir_pb.clone()),
             port: matches.value_of("port").map(u16::from_str).map(Result::unwrap),
-            follow_symlinks: !matches.is_present("no-follow-symlinks"),
+            follow_symlinks: follow_symlinks,
+            sandbox_symlinks: follow_symlinks && matches.is_present("sandbox-symlinks"),
             temp_directory: {
                 let (temp_s, temp_pb) = if let Some(tmpdir) = matches.value_of("temp-dir") {
                     (tmpdir.to_string(), fs::canonicalize(tmpdir).unwrap())
diff --git a/src/util/mod.rs b/src/util/mod.rs
index 9de27b1..850deb1 100644
--- a/src/util/mod.rs
+++ b/src/util/mod.rs
@@ -11,12 +11,12 @@ use std::borrow::Cow;
 use rfsapi::RawFileData;
 use url::percent_encoding;
 use iron::headers::UserAgent;
-use std::fs::{FileType, File};
 use std::collections::HashMap;
 use time::{self, Duration, Tm};
 use iron::{mime, Headers, Url};
 use std::io::{BufReader, BufRead};
 use base64::display::Base64Display;
+use std::fs::{self, FileType, File};
 use mime_guess::{guess_mime_type_opt, get_mime_type_str};
 
 pub use self::os::*;
@@ -219,6 +219,58 @@ pub fn is_actually_file(tp: &FileType) -> bool {
     tp.is_file() || is_device(tp)
 }
 
+/// Check if the specified path is a direct descendant (or an equal) of the specified path.
+pub fn is_descendant_of<Pw: AsRef<Path>, Po: AsRef<Path>>(who: Pw, of_whom: Po) -> bool {
+    let (mut who, of_whom) = if let Ok(p) = fs::canonicalize(who).and_then(|w| fs::canonicalize(of_whom).map(|o| (w, o))) {
+        p
+    } else {
+        return false;
+    };
+
+    if who == of_whom {
+        return true;
+    }
+
+    while let Some(who_p) = who.parent().map(|p| p.to_path_buf()) {
+        who = who_p;
+
+        if who == of_whom {
+            return true;
+        }
+    }
+
+    false
+}
+
+/// Check if the specified path is a direct descendant (or an equal) of the specified path, without without requiring it to
+/// exist in the first place.
+pub fn is_nonexistant_descendant_of<Pw: AsRef<Path>, Po: AsRef<Path>>(who: Pw, of_whom: Po) -> bool {
+    let mut who = fs::canonicalize(&who).unwrap_or_else(|_| who.as_ref().to_path_buf());
+    let of_whom = if let Ok(p) = fs::canonicalize(of_whom) {
+        p
+    } else {
+        return false;
+    };
+
+    if who == of_whom {
+        return true;
+    }
+
+    while let Some(who_p) = who.parent().map(|p| p.to_path_buf()) {
+        who = if let Ok(p) = fs::canonicalize(&who_p) {
+            p
+        } else {
+            who_p
+        };
+
+        if who == of_whom {
+            return true;
+        }
+    }
+
+    false
+}
+
 /// Construct string representing a human-readable size.
 ///
 /// Stolen, adapted and inlined from [fielsize.js](http://filesizejs.com).


Test patch: