From fbe5dbc7143121f2bf2ddbb8e089cb7aa3d86fa3 Mon Sep 17 00:00:00 2001
From: Dave Kempe <dave@sol1.com.au>
Date: Sun, 18 Oct 2026 10:38:53 +1100
Subject: [PATCH] vdi: refuse container usernames that are not plain names

The username is a directory under home_base that is bind-mounted into
the container. An override could carry ../ and the guard compared paths
as text, so it passed. container_username must now be 1-32 letters,
digits, '.', '_' or '-', not starting with '.' or '-'; the Docker driver
also requires exactly one path component and compares real paths, so a
symlink in home_base cannot redirect the mount either.
---
 src/session.rs    | 52 +++++++++++++++++++++++++++++++++++++++++++++++
 src/vdi/docker.rs | 28 ++++++++++++++++++++-----
 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/src/session.rs b/src/session.rs
index 681feb7..259799b 100644
--- a/src/session.rs
+++ b/src/session.rs
@@ -1591,6 +1591,18 @@ impl SessionManager {
                 // are scoped per-operator. When the override is set the same
                 // container is shared by everyone connecting with that entry,
                 // which is the desired behaviour for shared baked-in accounts.
+                // The username names the container and the home directory
+                // under `home_base`, so an override must be a plain name: a
+                // `../` here would bind-mount any host directory.
+                if let Some(name) = req.container_username.as_deref().filter(|s| !s.is_empty()) {
+                    if !valid_vdi_username(name) {
+                        return Err(SessionError::ValidationError(
+                            "container_username must be 1-32 letters, digits, '.', '_' or '-', \
+                             not starting with '.' or '-'"
+                                .into(),
+                        ));
+                    }
+                }
                 let vdi_username = req
                     .container_username
                     .as_ref()
@@ -2704,6 +2716,20 @@ fn drive_cleanup_settings(drive: &Option<DriveConfig>) -> (bool, u64) {
     }
 }
 
+/// A VDI container username: it becomes the container name suffix and a
+/// directory under `home_base`, so only a plain name is accepted. Letters,
+/// digits, '.', '_' and '-', at most 32 characters (the Linux limit), not
+/// starting with '.' or '-' (which rules out `.` and `..`).
+pub(crate) fn valid_vdi_username(name: &str) -> bool {
+    !name.is_empty()
+        && name.len() <= 32
+        && !name.starts_with('.')
+        && !name.starts_with('-')
+        && name
+            .chars()
+            .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-'))
+}
+
 #[derive(Debug)]
 pub enum SessionError {
     GuacdConnection(String),
@@ -2767,6 +2793,32 @@ pub(crate) fn check_share_token_match(
 mod tests {
     use super::*;
 
+    #[test]
+    fn vdi_username_must_be_a_plain_name() {
+        // It becomes a directory under home_base that is bind-mounted into the
+        // container, so anything that could leave home_base is refused.
+        for ok in ["alice", "svc.backup", "j_smith-2", "A1", &"a".repeat(32)] {
+            assert!(valid_vdi_username(ok), "{ok} should be accepted");
+        }
+        for bad in [
+            "",
+            "..",
+            ".",
+            "../../etc",
+            "../root",
+            "a/b",
+            "/etc",
+            ".hidden",
+            "-rf",
+            "a b",
+            "a\\b",
+            "é",
+            &"a".repeat(33),
+        ] {
+            assert!(!valid_vdi_username(bad), "{bad:?} should be refused");
+        }
+    }
+
     // ── Typescript filename templating (#159) ──
 
     fn ts_when() -> DateTime<Utc> {
diff --git a/src/vdi/docker.rs b/src/vdi/docker.rs
index 887767c..7fb05b8 100644
--- a/src/vdi/docker.rs
+++ b/src/vdi/docker.rs
@@ -560,15 +560,33 @@ impl DockerDriver {
                 // Persistent home directory bind mount
                 let binds = if let Some(ref base) = spec.home_base {
                     let base_path = std::path::Path::new(base);
-                    let host_path = base_path.join(&spec.username);
-                    // Verify no path traversal (username is sanitized but belt-and-suspenders)
-                    if !host_path.starts_with(base_path) {
-                        return Err(VdiError::Docker("path traversal in home_base".into()));
+                    // The username is validated where the session is made;
+                    // this is the second line of defence. A plain name has
+                    // exactly one normal path component, so `..`, `/` and an
+                    // absolute path never get this far.
+                    let mut parts = std::path::Path::new(&spec.username).components();
+                    if !matches!(
+                        (parts.next(), parts.next()),
+                        (Some(std::path::Component::Normal(_)), None)
+                    ) {
+                        return Err(VdiError::Docker(
+                            "invalid VDI username for home_base".into(),
+                        ));
                     }
-                    // Ensure host directory exists
+                    let host_path = base_path.join(&spec.username);
                     if let Err(e) = std::fs::create_dir_all(&host_path) {
                         tracing::warn!(path = ?host_path, "Failed to create VDI home dir: {}", e);
                     }
+                    // And compare real paths, so a symlink inside home_base
+                    // cannot point the mount somewhere else either.
+                    match (base_path.canonicalize(), host_path.canonicalize()) {
+                        (Ok(b), Ok(h)) if h.starts_with(&b) && h != b => {}
+                        _ => {
+                            return Err(VdiError::Docker(
+                                "VDI home directory is outside home_base".into(),
+                            ))
+                        }
+                    }
                     let mount = format!(
                         "{}:/home/{}:nosuid,nodev",
                         host_path.display(),