From 37ea5f616eae1163150c25ec99c41a9a086ee37a Mon Sep 17 00:00:00 2001
From: Christian Duerr <chris.durr@phylum.io>
Date: Thu, 28 Sep 2023 22:46:36 +0200
Subject: [PATCH] Fix directory leakage

---
 src/linux/namespaces.rs         | 87 ++++++++++++++++++++++++++++-----
 tests/net_without_namespaces.rs | 15 +++---
 2 files changed, 82 insertions(+), 20 deletions(-)

diff --git a/src/linux/namespaces.rs b/src/linux/namespaces.rs
index 129ab45..ac99025 100644
--- a/src/linux/namespaces.rs
+++ b/src/linux/namespaces.rs
@@ -7,7 +7,7 @@ use std::fs::{self, File};
 use std::io::Error as IoError;
 use std::os::unix::ffi::OsStrExt;
 use std::path::{Component, Path, PathBuf};
-use std::{env, ptr};
+use std::{env, io, ptr};
 
 use bitflags::bitflags;
 
@@ -88,17 +88,13 @@ fn create_mount_namespace(bind_mounts: HashMap<PathBuf, MountFlags>) -> Result<(
     });
 
     // Bind mount all allowed directories.
-    let current_dir = env::current_dir().ok();
-    for (mut path, flags) in bind_mounts {
-        // Ensure all paths are absolute.
-        if path.is_relative() {
-            let current_dir = match &current_dir {
-                Some(current_dir) => current_dir,
-                // Ignore relative paths if we cannot access the working directory.
-                None => continue,
-            };
-            path = current_dir.join(path);
-        }
+    for (path, flags) in bind_mounts {
+        // Ensure all paths are absolute, without following symlinks.
+        let path = match absolute(&path) {
+            Ok(path) => normalize_path(&path),
+            // Ignore relative paths if we cannot access the working directory.
+            Err(_) => continue,
+        };
 
         let src_c = CString::new(path.as_os_str().as_bytes()).unwrap();
 
@@ -374,3 +370,70 @@ bitflags! {
         const SYSVSEM = libc::CLONE_SYSVSEM;
     }
 }
+
+// Copied from Rust's STD:
+// https://github.com/rust-lang/rust/blob/42faef503f3e765120ca0ef06991337668eafc32/library/std/src/sys/unix/path.rs#L23C1-L63C2
+//
+// Licensed under MIT:
+// https://github.com/rust-lang/rust/blob/master/LICENSE-MIT
+//
+/// Make a POSIX path absolute without changing its semantics.
+fn absolute(path: &Path) -> io::Result<PathBuf> {
+    // This is mostly a wrapper around collecting `Path::components`, with
+    // exceptions made where this conflicts with the POSIX specification.
+    // See 4.13 Pathname Resolution, IEEE Std 1003.1-2017
+    // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13
+
+    // Get the components, skipping the redundant leading "." component if it
+    // exists.
+    let mut components = path.strip_prefix(".").unwrap_or(path).components();
+    let path_os = path.as_os_str().as_encoded_bytes();
+
+    let mut normalized = if path.is_absolute() {
+        // "If a pathname begins with two successive <slash> characters, the
+        // first component following the leading <slash> characters may be
+        // interpreted in an implementation-defined manner, although more than
+        // two leading <slash> characters shall be treated as a single <slash>
+        // character."
+        if path_os.starts_with(b"//") && !path_os.starts_with(b"///") {
+            components.next();
+            PathBuf::from("//")
+        } else {
+            PathBuf::new()
+        }
+    } else {
+        env::current_dir()?
+    };
+    normalized.extend(components);
+
+    // "Interfaces using pathname resolution may specify additional constraints
+    // when a pathname that does not name an existing directory contains at
+    // least one non- <slash> character and contains one or more trailing
+    // <slash> characters".
+    // A trailing <slash> is also meaningful if "a symbolic link is
+    // encountered during pathname resolution".
+    if path_os.ends_with(b"/") {
+        normalized.push("");
+    }
+
+    Ok(normalized)
+}
+
+/// Normalize path components, stripping out `.` and `..`.
+fn normalize_path(path: &Path) -> PathBuf {
+    let mut normalized = PathBuf::new();
+
+    for component in path.components() {
+        match component {
+            Component::Prefix(_) => unreachable!("impl does not consider windows"),
+            Component::RootDir => normalized.push("/"),
+            Component::CurDir => continue,
+            Component::ParentDir => {
+                normalized.pop();
+            },
+            Component::Normal(segment) => normalized.push(segment),
+        }
+    }
+
+    normalized
+}
diff --git a/tests/net_without_namespaces.rs b/tests/net_without_namespaces.rs
index 58216a7..88dce6d 100644
--- a/tests/net_without_namespaces.rs
+++ b/tests/net_without_namespaces.rs
@@ -33,15 +33,14 @@ fn main() {
     let birdcage = Birdcage::new().unwrap();
     let result = birdcage.lock();
 
-    match result {
-        // Seccomp is supported, so networking should still be blocked.
-        Ok(_) => {
-            let result = TcpStream::connect("8.8.8.8:443");
-            assert!(result.is_err());
-        },
-        // Seccomp isn't supported, so failure is desired.
-        Err(_) => (),
+    // Seccomp isn't supported, so failure is desired.
+    if result.is_err() {
+        return;
     }
+
+    // Seccomp is supported, so networking should still be blocked.
+    let result = TcpStream::connect("8.8.8.8:443");
+    assert!(result.is_err());
 }
 
 #[cfg(not(target_os = "linux"))]