From 984270fbecdaace2353d8247f04b94626f10390f Mon Sep 17 00:00:00 2001 From: ljkx <177671075+ljkx@users.noreply.github.com> Date: Mon, 9 Nov 2026 12:38:29 +0100 Subject: [PATCH] M8: automation roots refuse a path on another computer before resolving it (#1094) On Windows, Roots::check resolved every path (canonicalize opens it) before comparing it with the roots, so `\server\share\x` contacted the server, and could send it the user's credentials, before it was refused. Now the path as given and made absolute is classified by its spelling with craft_docpath first: another computer that no root's share is on, a device path, or a DOS device name anywhere in it is refused, and so is a link on the way that leads to one (followed by its target's spelling, up to 40 links). Fixes #1094 --- crates/engine/src/automation/file_access.rs | 189 ++++++++++++++++-- .../src/automation/file_access/tests.rs | 177 ++++++++++++++++ 2 files changed, 344 insertions(+), 22 deletions(-) diff --git a/crates/engine/src/automation/file_access.rs b/crates/engine/src/automation/file_access.rs index 5ce06b122..97c6cd2f5 100644 --- a/crates/engine/src/automation/file_access.rs +++ b/crates/engine/src/automation/file_access.rs @@ -12,6 +12,11 @@ //! existing folder is resolved and the rest must be plain names. The comparison is by whole //! components (`/work2` is not inside `/work`), and the I/O uses the resolved path, so what was //! checked is what is opened. +//! +//! On Windows, resolving a path opens it, and opening a path on another computer contacts that +//! computer and may send it the signed-in user's credentials. So before anything is resolved, a +//! path on another computer that no root is on, a device path, or one reached through a link that +//! leads to them is refused by how it is spelled (#1094). #![allow(clippy::disallowed_methods)] use std::cell::RefCell; @@ -19,6 +24,8 @@ use std::ffi::{OsStr, OsString}; use std::path::{Component, Path, PathBuf}; use std::sync::Arc; +use craft_docpath::Refused; + use crate::{EngineError, Result}; /// What a command does with a file. @@ -58,6 +65,19 @@ impl Roots { Access::Read => ([self.read.as_slice(), self.write.as_slice()].concat(), "read"), Access::Write => (self.write.clone(), "written"), }; + if cfg!(windows) { + match refused_unresolved(path, &roots) { + Some(Unresolved::Spelled(Refused::Network)) => { + return Err(format!( + "automation path rejected: {asked} is on another computer, and no folder that may be {what} is on its share" + )); + } + Some(Unresolved::Spelled(_)) => return Err(format!("automation path rejected: {asked} names a device, not a file")), + // Where a link leads is only told by the usual message: it would say what is there. + Some(Unresolved::Linked) => return Err(outside(asked, &roots, what)), + None => {} + } + } match resolve(path) { Ok(resolved) if roots.iter().any(|r| resolved.starts_with(r)) => return Ok(resolved), Ok(_) => {} @@ -65,11 +85,7 @@ impl Roots { Err(why) if inside_as_far_as_it_exists(path, &roots) => return Err(format!("automation path rejected: {asked}: {why}")), Err(_) => {} } - if roots.is_empty() { - return Err(format!("automation path rejected: no folder may be {what} (no root for it was given): {asked}")); - } - let shown: Vec = roots.iter().map(|r| shown(r)).collect(); - Err(format!("automation path rejected: {asked} is outside the folders that may be {what}: {}", shown.join(", "))) + Err(outside(asked, &roots, what)) } /// The folders as JSON, resolved: `{read: [every folder that may be read], write: [the write roots]}`. @@ -79,23 +95,146 @@ impl Roots { } } +/// Why `asked` is refused: it is outside every one of `roots`, the folders that may be `what`. +fn outside(asked: std::path::Display<'_>, roots: &[PathBuf], what: &str) -> String { + if roots.is_empty() { + return format!("automation path rejected: no folder may be {what} (no root for it was given): {asked}"); + } + let shown: Vec = roots.iter().map(|r| shown(r)).collect(); + format!("automation path rejected: {asked} is outside the folders that may be {what}: {}", shown.join(", ")) +} + +/// Why a path is refused before it is resolved (#1094). +#[derive(Debug, PartialEq, Eq)] +enum Unresolved { + /// Its spelling leads to another computer ([`Refused::Network`]) or a device ([`Refused::Device`]). + Spelled(Refused), + /// A link on the way leads there. + Linked, +} + +/// How many links are followed on the way to a path before it is given up on, as on Linux. +const MAX_LINKS: u8 = 40; + +/// Why `path` is refused before anything on the way to it is opened, if it is: resolving it would +/// open a path on another computer, which on Windows contacts that computer and may send it the +/// signed-in user's credentials, or open a device (#1094). That is the case when the path as +/// given or made absolute is (see [`refused_spelling`]), or when a link on the way, or on the way +/// on from where one leads, leads to such a path ([`link_refused`]). A path on the share of one of +/// `roots` goes on to the usual check. +fn refused_unresolved(path: &Path, roots: &[PathBuf]) -> Option { + // Made absolute without looking at the disk (on Windows `GetFullPathNameW`, which also turns a + // bare device name such as `NUL` into `\\.\NUL`, as opening it would). + let abs = std::path::absolute(path).ok()?; + if let Some(why) = [path, abs.as_path()].into_iter().find_map(|p| refused_spelling(&p.to_string_lossy(), roots)) { + return Some(Unresolved::Spelled(why)); + } + link_refused(&abs, roots, MAX_LINKS, &on_disk).then_some(Unresolved::Linked) +} + +/// What is at a path, looked at without following a link there. +#[derive(Debug, Clone, PartialEq, Eq)] +enum Entry { + /// Nothing, or what can't be looked at: resolving the path meets the same there and goes no + /// further. + Missing, + /// A file or a folder. + Plain, + /// A link, with where it leads (`None` when that can't be read). + Link(Option), +} + +/// What is at `p` on disk (see [`Entry`]). +fn on_disk(p: &Path) -> Entry { + match std::fs::symlink_metadata(p) { + Ok(m) if m.is_symlink() => Entry::Link(std::fs::read_link(p).ok()), + Ok(_) => Entry::Plain, + Err(_) => Entry::Missing, + } +} + +/// Why path `p` is refused by its spelling alone, if it is (see [`craft_docpath::resolve`], which +/// reads both `/` and `\` as separators and compares names ignoring case): +/// - on another computer (`\\server\share`, `//server/share`, `\\?\UNC\…`, `\\.\UNC\…`, or a +/// server without a share) and on no root's share; +/// - a device path (`\\.\pipe\…`, `\\?\GLOBALROOT\…`, `\\.\C:\…`), other than a local volume's +/// `\\?\Volume{…}\`, which a folder can be mounted as; +/// - or with a device name (`CON`, `NUL`, `COM1.png`, `lpt1 `…) as any of its names: each folder on +/// the way is opened in turn while the path is resolved, and the device behind a name such as +/// `LPT1` can be a printer on another computer. +fn refused_spelling(p: &str, roots: &[PathBuf]) -> Option { + match craft_docpath::resolve(p, None) { + // With a root as the document, a path on its share is allowed. + Err(Refused::Network) if !roots.iter().any(|r| craft_docpath::resolve(p, Some(r)) != Err(Refused::Network)) => { + return Some(Refused::Network); + } + Err(Refused::Device) if !is_volume(p) => return Some(Refused::Device), + _ => {} + } + // `craft_docpath` stops at the first name it refuses, which may not be the device's. + p.split(['\\', '/']).any(device_name).then_some(Refused::Device) +} + +/// Whether `p` is in a local volume by its `\\?\Volume{…}` (or `\\.\Volume{…}`) name. +fn is_volume(p: &str) -> bool { + let mut names = p.split(['\\', '/']); + let (Some(""), Some(""), Some("?" | "."), Some(volume)) = (names.next(), names.next(), names.next(), names.next()) else { + return false; + }; + volume.get(..7).is_some_and(|v| v.eq_ignore_ascii_case("Volume{")) && volume.ends_with('}') +} + +/// Whether a link on the way to `abs` (an absolute path) leads to a path [`refused_spelling`] +/// refuses, following at most `links` links: one that leads to another, or on through further +/// links, is followed too, as resolving the path would. A link that can't be read, or more links +/// than that, are refused as well. Each name on the way is looked at with `look` ([`on_disk`]), +/// which doesn't follow a link: where a link leads is looked at only once its spelling is known +/// not to be refused. +fn link_refused(abs: &Path, roots: &[PathBuf], links: u8, look: &dyn Fn(&Path) -> Entry) -> bool { + let mut at = PathBuf::new(); + let mut rest = abs.components(); + while let Some(c) = rest.next() { + at.push(c); + if !matches!(c, Component::Normal(_)) { + continue; + } + let target = match look(&at) { + Entry::Plain => continue, + // Nothing is below what isn't there, so no link either. + Entry::Missing => return false, + Entry::Link(target) => target, + }; + let (Some(links), Some(target)) = (links.checked_sub(1), target) else { return true }; + // A relative target is relative to the link's folder; an absolute one replaces it. + let next = lexical(&at.parent().unwrap_or(&at).join(target).join(rest.as_path())); + return refused_spelling(&next.to_string_lossy(), roots).is_some() || link_refused(&next, roots, links, look); + } + false +} + +/// `p` with its `.` and `..` taken by name, as Windows takes them. +fn lexical(p: &Path) -> PathBuf { + let mut out = PathBuf::new(); + for c in p.components() { + match c { + Component::ParentDir => { + out.pop(); + } + Component::CurDir => {} + c => out.push(c), + } + } + out +} + /// Whether `path` lies in one of `roots` once made absolute with its `.` and `..` taken by name and /// its longest leading part that exists resolved, so links on the way are followed as the roots' /// were (`/tmp/work/x` is inside root `/private/tmp/work`, a link out of a root leads out of it). fn inside_as_far_as_it_exists(path: &Path, roots: &[PathBuf]) -> bool { let Ok(abs) = std::path::absolute(path) else { return false }; - let mut lexical = PathBuf::new(); - for c in abs.components() { - match c { - Component::ParentDir => { - lexical.pop(); - } - Component::CurDir => {} - c => lexical.push(c), - } - } + let folded = lexical(&abs); let mut rest: Vec<&OsStr> = Vec::new(); - let mut at = lexical.as_path(); + let mut at = folded.as_path(); let resolved = loop { if let Ok(mut base) = std::fs::canonicalize(at) { base.extend(rest.iter().rev()); @@ -169,18 +308,24 @@ fn resolve(path: &Path) -> std::result::Result { fn plain_name(name: &OsStr) -> std::result::Result<(), String> { if cfg!(windows) { let n = name.to_string_lossy(); - let stem = n.split('.').next().unwrap_or_default().trim_end().to_ascii_uppercase(); - let device = matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL" | "CONIN$" | "CONOUT$") - || ((stem.starts_with("COM") || stem.starts_with("LPT")) - && stem.chars().count() == 4 - && stem.chars().last().is_some_and(|c| c.is_ascii_digit() || matches!(c, '¹' | '²' | '³'))); - if device || n.contains(':') { + if device_name(&n) || n.contains(':') { return Err(format!("`{n}` is a device or stream name, not a file")); } } Ok(()) } +/// Whether Windows opens a device for file or folder name `n`: `CON`, `PRN`, `AUX`, `NUL`, +/// `CONIN$`, `CONOUT$`, `COM1`…`COM9` and `LPT1`…`LPT9` (also with `¹`, `²` or `³`), in any case, +/// and with anything after a dot or a colon or trailing spaces (`nul.txt`, `COM1 .png`, `con:x`). +fn device_name(n: &str) -> bool { + let stem = n.split(['.', ':']).next().unwrap_or_default().trim_end().to_ascii_uppercase(); + matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL" | "CONIN$" | "CONOUT$") + || ((stem.starts_with("COM") || stem.starts_with("LPT")) + && stem.chars().count() == 4 + && stem.chars().last().is_some_and(|c| c.is_ascii_digit() || matches!(c, '¹' | '²' | '³'))) +} + thread_local! { static ROOTS: RefCell>> = const { RefCell::new(None) }; } diff --git a/crates/engine/src/automation/file_access/tests.rs b/crates/engine/src/automation/file_access/tests.rs index 72f471f94..5f9c1ad5d 100644 --- a/crates/engine/src/automation/file_access/tests.rs +++ b/crates/engine/src/automation/file_access/tests.rs @@ -191,6 +191,183 @@ fn windows_case_devices_and_streams() { assert!(!e.contains(r"\\?\"), "{e}"); } +/// How each spelling of a path on another computer or of a device is told apart from a local path +/// (#1094), on every system: only Windows checks them, but they are spelled the same everywhere. +#[test] +fn spellings_of_other_computers_and_devices_are_refused() { + use super::refused_spelling; + use craft_docpath::Refused::{Device, Network}; + let local = [PathBuf::from(r"C:\work")]; + let network = [ + r"\\192.0.2.1\share\x.png", + "//192.0.2.1/share/x.png", + r"\/192.0.2.1/share\x.png", + r"\\?\UNC\192.0.2.1\share\x.png", + r"\\?\unc\192.0.2.1\share\x.png", + "//?/UNC/192.0.2.1/share/x.png", + r"\\.\UNC\192.0.2.1\share\x.png", + // A server without a share is contacted too. + r"\\192.0.2.1", + r"\\192.0.2.1\", + r"\\SERVER.\share \x.png", + ]; + let device = [ + r"\\.\pipe\figcraft", + r"\\?\GLOBALROOT\Device\Mup\192.0.2.1\share\x.png", + r"\\.\C:\work\x.png", + r"\\.\COM1", + r"\??\UNC\192.0.2.1\share\x.png", + // A device name anywhere: each folder on the way is opened as the path is resolved. + r"C:\work\COM1", + r"C:\work\nul.txt", + r"C:\work\Com1 .png", + r"C:\work\lpt1.\x.png", + r"C:\work\LPT³\x.png", + r"C:\work\conout$", + r"C:\work\aux:stream", + // After a name `craft_docpath` refuses for another reason. + r"C:\work\a:b\prn.png", + ]; + let fine = [ + r"C:\work\x.png", + r"\\?\C:\work\x.png", + r"C:\work\console.png", + r"C:\work\com10.png", + r"C:\work\lpt.png", + r"C:\work\nul-ish.txt", + r"\\?\Volume{0b3b1c63-0000-0000-0000-100000000000}\x.png", + "/home/u/work/x.png", + "x.png", + r"..\x.png", + ]; + for (paths, want) in [(&network[..], Some(Network)), (&device[..], Some(Device)), (&fine[..], None)] { + for p in paths { + assert_eq!(refused_spelling(p, &[]), want, "{p}"); + assert_eq!(refused_spelling(p, &local), want, "{p} with a local root"); + } + } + // A root on a share lets the paths on that share through, in any case and spelling, and only them. + let shared = [PathBuf::from(r"\\?\UNC\Server\Share\work")]; + for p in [r"\\server\SHARE\work\x.png", r"\\?\UNC\SERVER\share\elsewhere\x.png", "//Server/Share/work/x.png"] { + assert_eq!(refused_spelling(p, &shared), None, "{p}"); + } + for p in [r"\\server\other\x.png", r"\\other\share\x.png", r"\\server", r"\\server.\share\x.png"] { + assert_eq!(refused_spelling(p, &shared), Some(Network), "{p}"); + } + assert_eq!(refused_spelling(r"\\server\share\work\COM1", &shared), Some(Device)); +} + +/// The links on the way to a path are followed by their spelling before anything they lead to is +/// opened (#1094). The links are made up here, so this runs where tests may not make links. +#[test] +fn links_on_the_way_are_followed_by_their_spelling_first() { + use super::{Entry, MAX_LINKS, link_refused}; + use std::cell::RefCell; + use std::collections::HashMap; + let base = std::env::temp_dir().join("figcraft-links"); + let at = |rel: &str| base.join(rel); + let mut links: HashMap> = HashMap::from([ + // Relative to the link's folder, to a device name. + (at("root/dev"), Some(PathBuf::from("../outside/nul"))), + // To another link. + (at("root/hop"), Some(at("root/dev"))), + // Within the root. + (at("root/alias"), Some(PathBuf::from("sub"))), + (at("root/loop"), Some(at("root/loop"))), + (at("root/unreadable"), None), + // Below a name that isn't there: never looked at. + (at("missing/dev"), Some(PathBuf::from("nul"))), + ]); + if cfg!(windows) { + links.insert(at("root/remote"), Some(PathBuf::from(r"\\192.0.2.1\share"))); + links.insert(at("root/far"), Some(at("root/remote/work"))); + } + let plain = [at("root"), at("root/sub"), at("outside")]; + let looked = RefCell::new(Vec::new()); + let look = |p: &Path| { + looked.borrow_mut().push(p.to_path_buf()); + match links.get(p) { + Some(target) => Entry::Link(target.clone()), + None if base.starts_with(p) || plain.iter().any(|q| q == p) => Entry::Plain, + None => Entry::Missing, + } + }; + let refused = |rel: &str, roots: &[PathBuf]| link_refused(&at(rel), roots, MAX_LINKS, &look); + for rel in ["root/dev/x.png", "root/hop/x.png", "root/loop/x.png", "root/unreadable/x.png"] { + assert!(refused(rel, &[]), "{rel}"); + } + for rel in ["root/alias/x.png", "root/sub/x.png", "root/x.png", "missing/dev/x.png"] { + assert!(!refused(rel, &[]), "{rel}"); + } + // A refused target isn't looked at, nor is anything below what isn't there. + let looked = looked.take(); + assert!(!looked.contains(&at("outside/nul")) && !looked.contains(&at("missing/dev")), "{looked:?}"); + if cfg!(windows) { + for rel in ["root/remote/x.png", "root/far/x.png"] { + assert!(refused(rel, &[]), "{rel}"); + // Unless a root is on that share. + assert!(!refused(rel, &[PathBuf::from(r"\\?\UNC\192.0.2.1\share")]), "{rel}"); + } + } +} + +/// A path on another computer, a device path, and a link to them are refused before anything is +/// opened: resolving them made Windows contact the server, which may send it the signed-in user's +/// credentials (#1094). Nothing answers on 192.0.2.1 (an address for documentation), so a path +/// there that is opened takes Windows seconds to give up on; one refused first, none. +#[cfg(windows)] +#[test] +fn other_computers_and_devices_are_refused_before_anything_is_opened() { + use std::time::{Duration, Instant}; + let f = Fixture::new("network"); + let r = f.rw(); + let read_only = Roots::new(&[f.p("root")], &[]).unwrap(); + let refused = |roots: &Roots, p: &str, access: Access| { + let t = Instant::now(); + let e = roots.check(Path::new(p), access).expect_err(p); + assert!(t.elapsed() < Duration::from_secs(3), "{p}: took {:?}, so it was opened", t.elapsed()); + e + }; + for p in + [r"\\192.0.2.1\share\x.png", "//192.0.2.1/share/x.png", r"\\?\UNC\192.0.2.1\share\x.png", r"\\.\UNC\192.0.2.1\share\x.png", r"\\192.0.2.1"] + { + for access in [Access::Read, Access::Write] { + let e = refused(&r, p, access); + assert!(e.contains("is on another computer, and no folder that may be"), "{p}: {e}"); + } + let e = refused(&read_only, p, Access::Write); + assert!(e.ends_with("is on another computer, and no folder that may be written is on its share"), "{p}: {e}"); + } + for p in [r"\\.\pipe\figcraft-roots", r"\\?\GLOBALROOT\Device\Mup\192.0.2.1\share\x.png", &f.p(r"root\COM1\x.png"), "nul"] { + let e = refused(&r, p, Access::Read); + assert!(e.ends_with("names a device, not a file"), "{p}: {e}"); + } + // Local paths, in either spelling, are checked as before. + let inside = std::fs::canonicalize(f.p("root")).unwrap().join("in.txt"); + assert!(r.check(&inside, Access::Read).is_ok()); + assert!(r.check(Path::new(&f.p(r"root\in.txt")), Access::Read).is_ok()); + + // A link in a root to another computer is refused before it is followed, also through another + // link, and with the message any path outside gets: it doesn't tell where the link leads. + if !symlink_dir(Path::new(r"\\192.0.2.1\share"), &f.0.join("root/remote")) || !symlink_dir(&f.0.join("root/remote/work"), &f.0.join("root/hop")) { + eprintln!("skipped links: this system doesn't let tests make symbolic links"); + return; + } + // Relative to the link's folder. + assert!(symlink_dir(Path::new(r"..\remote"), &f.0.join("root/sub/up"))); + assert!(symlink_file(Path::new(r"\\.\pipe\figcraft-roots"), &f.0.join("root/pipe"))); + let standard = |p: &str| format!("automation path rejected: {p} is outside the folders that may be"); + for p in [f.p("root/remote/x.png"), f.p("root/hop/x.png"), f.p("root/sub/up/x.png"), f.p("root/pipe")] { + for access in [Access::Read, Access::Write] { + let e = refused(&r, &p, access); + assert!(e.starts_with(&standard(&p)), "{p}: {e}"); + } + } + // A link to a folder in the root still leads there. + assert!(symlink_dir(&f.0.join("root/sub"), &f.0.join("root/alias"))); + assert!(r.check(Path::new(&f.p("root/alias/x.png")), Access::Write).is_ok()); +} + #[test] fn a_scope_confines_the_thread_and_ends_with_it() { let f = Fixture::new("scope");