agate

commit 3c38dae5995dcc17c8b201595990ef82d741f811
parent aeba1974fedde4b7149f141c1a832911939ff373
Author: Matthew Ingwersen <matttpt@gmail.com>
Date:   Mon,  7 Jun 2021 20:52:23 -0400

Fix directory traversal vulnerability

When computing the filesystem path to serve, each URL path segment
appended to the content directory path must be checked to ensure that it
consists only of normal filesystem path components (and not the root
directory, .., drive labels, or other special components). Otherwise,
the following directory traversal attacks are possible:

- When an absolute path is pushed onto a PathBuf, the PathBuf will be
  overwritten. If we don't check for absolute paths, Agate can be
  tricked into serving an arbitrary absolute filesystem path via a URL
  like gemini://example.com/%2Fetc/passwd

- The url crate eliminates all .. segments from the URL when parsing,
  even when these are percent-encoded. However, .. can be injected
  into the computed filesystem path by using a URL path segment that,
  when decoded, contains more than one filesystem path component, like
  gemini://example.com/subdir%2F..%2F../outside_content_dir

Furthermore, path separators appearing within a single URL path segment,
like escaped / (%2F), should probably not be considered structural [0].
That is, "a%2Fb" refers to a resource literally named "a/b", not "b" in
subdirectory "a". Thus we also check that a URL path segment represents
no more than one filesystem path segment.

[0] https://www.w3.org/Addressing/URL/4_URI_Recommentations.html

Diffstat:
M
src/main.rs
 | 
37
++++++++++++++++++++++++++++++-------

1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/src/main.rs b/src/main.rs
@@ -17,7 +17,7 @@ use {
         fs::{self, File},
         io::Write as _,
         net::SocketAddr,
-        path::{Path, PathBuf},
+        path::{self, Component, Path, PathBuf},
         sync::Arc,
     },
     tokio::{
@@ -433,12 +433,35 @@ impl RequestHandle {
 
         if let Some(mut segments) = url.path_segments() {
             // append percent-decoded path segments
-            path.extend(
-                segments
-                    .clone()
-                    .map(|segment| Ok(percent_decode_str(segment).decode_utf8()?.into_owned()))
-                    .collect::<Result<Vec<_>>>()?,
-            );
+            for segment in segments.clone() {
+                // To prevent directory traversal attacks, we need to
+                // check that each filesystem path component in the URL
+                // path segment is a normal component (not the root
+                // directory, the parent directory, a drive label, or
+                // another special component). Furthermore, since path
+                // separators (e.g. the escaped forward slash %2F) in a
+                // single URL path segment are non-structural, the URL
+                // path segment should not contain multiple filesystem
+                // path components.
+                let decoded = percent_decode_str(segment).decode_utf8()?;
+                let mut components = Path::new(decoded.as_ref()).components();
+                // the first component must be a normal component; if
+                // so, push it onto the PathBuf
+                match components.next() {
+                    None => (),
+                    Some(Component::Normal(c)) => path.push(c),
+                    Some(_) => return self.send_header(51, "Not found, sorry.").await,
+                }
+                // there must not be more than one component
+                if components.next().is_some() {
+                    return self.send_header(51, "Not found, sorry.").await;
+                }
+                // even if it's one component, there may be trailing path
+                // separators at the end
+                if decoded.ends_with(path::is_separator) {
+                    return self.send_header(51, "Not found, sorry.").await;
+                }
+            }
             // check if hiding files is disabled
             if !ARGS.serve_secret
                 // there is a configuration for this file, assume it should be served