diff --git a/crates/icp-project/src/files.rs b/crates/icp-project/src/files.rs index 860cee6a9..e9b4936d8 100644 --- a/crates/icp-project/src/files.rs +++ b/crates/icp-project/src/files.rs @@ -88,6 +88,202 @@ pub trait Scratch: Send + Sync { fn path(&self) -> &Path; } +/// This machine's filesystem, held to one directory, for a project that is +/// not the user's own tree: one handed over by someone else. +/// +/// Every path must name something under the root, reached through no +/// symlink. A relative path is joined to the root, `.` is dropped, `..` is +/// applied, and a path that comes to rest outside the root is refused with +/// [`OutsideRoot`]. Each component below the root is then looked at as it is +/// on disk, and a path that passes through a symlink is refused with +/// [`ThroughLink`], wherever the link leads, even where a later `..` steps +/// back out of it. The predicates answer `false` for a refused path, and +/// scratch directories are made under the root. +/// +/// This confines what is read and written through it, and nothing else: a +/// `script` build step in a manifest still runs any command, so whoever +/// holds a tree they do not trust must also refuse such steps or sandbox them. +/// +/// The loader reads every path through the [`FileSystem`] it is given, so +/// this holds all of them. A local recipe template is the exception: the +/// [`Resolve`](crate::canister::recipe::Resolve) implementation reads it, and +/// must read it through this filesystem or refuse it. +/// +/// Known issue: a predicate can only answer `true` or `false`, so the reason +/// a path was refused is lost there, and a caller that checks before it +/// reads reports a refused path as missing. The loader does this for a +/// canister directory and for a dependency's manifest: `canisters: +/// [../elsewhere]` fails as "not found" rather than as outside the root. +/// +/// The root is absolute, and is matched as the caller spelled it, not as it +/// canonicalizes. It and everything above it are the caller's, and may be +/// reached through a link; only what lies below is held to shape. The tree +/// is taken to be the caller's own while it is used, with nothing else +/// rewriting it between the look and the read. +#[cfg(feature = "host")] +#[derive(Debug, Clone)] +pub struct Confined { + root: PathBuf, +} + +/// A root a [`Confined`] filesystem was asked to hold to that is not absolute. +#[cfg(feature = "host")] +#[derive(Debug, Snafu)] +#[snafu(display( + "'{root}' is not an absolute path, which the directory a project is confined to must be" +))] +pub struct RootNotAbsolute { + pub root: PathBuf, +} + +/// A path a [`Confined`] filesystem was asked for that is not under its root. +#[cfg(feature = "host")] +#[derive(Debug, Snafu)] +#[snafu(display("'{path}' is outside '{root}', the directory this project is confined to"))] +pub struct OutsideRoot { + pub path: PathBuf, + pub root: PathBuf, +} + +/// A path a [`Confined`] filesystem was asked for that passes through a +/// symlink. +#[cfg(feature = "host")] +#[derive(Debug, Snafu)] +#[snafu(display( + "'{path}' passes through '{link}', which is a symlink, and every path in this project must \ + name a regular file or directory under its root" +))] +pub struct ThroughLink { + pub path: PathBuf, + pub link: PathBuf, +} + +#[cfg(feature = "host")] +impl Confined { + /// Confine the host filesystem to `root`, which must be absolute. + pub fn new(root: &Path) -> Result { + snafu::ensure!(root.is_absolute(), RootNotAbsoluteSnafu { root }); + Ok(Self { + root: root.to_path_buf(), + }) + } + + /// The directory everything is held to. + pub fn root(&self) -> &Path { + &self.root + } + + /// Where `path` lies under the root: joined to the root if relative, + /// with `.` dropped and `..` applied, and refused if it would come to + /// rest outside, or passes through a symlink on the way. What a caller + /// that needs a real path for something that does not read through the + /// trait — a runtime mounting a directory — asks for. + pub fn resolve(&self, path: &Path) -> Result { + let outside = || { + FsError::new(OutsideRoot { + path: path.to_path_buf(), + root: self.root.clone(), + }) + }; + let relative = if path.is_absolute() { + path.strip_prefix(&self.root).map_err(|_| outside())? + } else { + path + }; + // Each component is looked at as it is on disk the moment it is + // named, before any `..` after it is applied: the host would follow a + // link and take `..` from where it leads, so a path that so much as + // mentions one is refused, dangling or not. + let mut resolved = self.root.clone(); + for component in relative.components() { + match component { + Utf8Component::Normal(part) => { + resolved.push(part); + if std::fs::symlink_metadata(&resolved) + .is_ok_and(|m| m.file_type().is_symlink()) + { + return Err(FsError::new(ThroughLink { + path: path.to_path_buf(), + link: resolved, + })); + } + } + Utf8Component::CurDir => {} + Utf8Component::ParentDir => { + if resolved == self.root || !resolved.pop() { + return Err(outside()); + } + } + Utf8Component::RootDir | Utf8Component::Prefix(_) => return Err(outside()), + } + } + Ok(resolved) + } +} + +#[cfg(feature = "host")] +#[async_trait] +impl FileSystem for Confined { + async fn read(&self, path: &Path) -> Result, FsError> { + HostFileSystem.read(&self.resolve(path)?).await + } + + async fn read_to_string(&self, path: &Path) -> Result { + HostFileSystem.read_to_string(&self.resolve(path)?).await + } + + async fn write(&self, path: &Path, contents: &[u8]) -> Result<(), FsError> { + HostFileSystem.write(&self.resolve(path)?, contents).await + } + + async fn create_dir_all(&self, path: &Path) -> Result<(), FsError> { + HostFileSystem.create_dir_all(&self.resolve(path)?).await + } + + async fn copy(&self, from: &Path, to: &Path) -> Result<(), FsError> { + HostFileSystem + .copy(&self.resolve(from)?, &self.resolve(to)?) + .await + } + + async fn exists(&self, path: &Path) -> bool { + match self.resolve(path) { + Ok(path) => HostFileSystem.exists(&path).await, + Err(_) => false, + } + } + + async fn is_file(&self, path: &Path) -> bool { + match self.resolve(path) { + Ok(path) => HostFileSystem.is_file(&path).await, + Err(_) => false, + } + } + + async fn is_dir(&self, path: &Path) -> bool { + match self.resolve(path) { + Ok(path) => HostFileSystem.is_dir(&path).await, + Err(_) => false, + } + } + + async fn read_dir(&self, path: &Path) -> Result, FsError> { + HostFileSystem.read_dir(&self.resolve(path)?).await + } + + async fn canonicalize(&self, path: &Path) -> Option { + HostFileSystem.canonicalize(&self.resolve(path).ok()?).await + } + + async fn scratch_dir(&self) -> Result, FsError> { + let dir = camino_tempfile::Builder::new() + .prefix(".scratch-") + .tempdir_in(&self.root) + .map_err(FsError::new)?; + Ok(Box::new(HostScratch(dir))) + } +} + #[cfg(feature = "host")] /// The [`FileSystem`] backed by this machine's filesystem. #[derive(Debug, Default, Clone, Copy)] @@ -385,6 +581,193 @@ async fn list( mod tests { use super::*; + /// Every spelling of a path resolves under the root or is refused: a + /// relative one is joined, `.` and `..` are applied, an absolute one is + /// taken only under the root, and none may pass through a symlink. + #[cfg(feature = "host")] + mod confined { + use super::*; + + fn root() -> (camino_tempfile::Utf8TempDir, Confined) { + let dir = camino_tempfile::Utf8TempDir::new().expect("temp dir"); + std::fs::create_dir_all(dir.path().join("project/canisters")).expect("mkdir"); + std::fs::write(dir.path().join("project/icp.yaml"), b"canisters: []").expect("write"); + std::fs::write(dir.path().join("project/canisters/a.wasm"), b"wasm").expect("write"); + std::fs::write(dir.path().join("outside.txt"), b"secret").expect("write"); + let fs = Confined::new(&dir.path().join("project")).expect("absolute"); + (dir, fs) + } + + #[tokio::test] + async fn paths_resolve_under_the_root_by_any_spelling_inside_it() { + let (_dir, fs) = root(); + let root = fs.root().to_path_buf(); + for spelling in [ + "canisters/a.wasm", + "./canisters/a.wasm", + "canisters/../canisters/a.wasm", + root.join("canisters/a.wasm").as_str(), + ] { + assert_eq!(fs.read(Path::new(spelling)).await.expect(spelling), b"wasm"); + assert!(fs.is_file(Path::new(spelling)).await, "{spelling}"); + assert_eq!( + fs.canonicalize(Path::new(spelling)).await, + HostFileSystem + .canonicalize(&root.join("canisters/a.wasm")) + .await, + "{spelling}" + ); + } + assert!(fs.is_dir(Path::new("canisters")).await); + assert!(fs.is_dir(&root).await); + assert_eq!( + fs.read_dir(&root).await.expect("list"), + vec![root.join("canisters"), root.join("icp.yaml")] + ); + assert!( + fs.resolve(Path::new("made/later")) + .unwrap() + .starts_with(&root) + ); + } + + #[tokio::test] + async fn paths_outside_the_root_are_refused_however_spelled() { + let (dir, fs) = root(); + let outside = dir.path().join("outside.txt"); + assert!( + outside.is_file(), + "the file exists, for the host filesystem to read" + ); + for spelling in [ + "../outside.txt", + "canisters/../../outside.txt", + outside.as_str(), + "/etc/hostname", + "../../../../../../etc/hostname", + ] { + let err = fs + .read(Path::new(spelling)) + .await + .expect_err(spelling) + .to_string(); + assert!(err.contains("is outside"), "{spelling}: {err}"); + assert!(!fs.exists(Path::new(spelling)).await, "{spelling}"); + assert!(!fs.is_file(Path::new(spelling)).await, "{spelling}"); + assert!( + fs.canonicalize(Path::new(spelling)).await.is_none(), + "{spelling}" + ); + assert!(fs.resolve(Path::new(spelling)).is_err(), "{spelling}"); + } + // Nor may anything be written there. + let err = fs + .write(Path::new("../planted"), b"x") + .await + .unwrap_err() + .to_string(); + assert!(err.contains("is outside"), "{err}"); + assert!(!dir.path().join("planted").exists()); + } + + /// A temp dir on macOS is reached through a symlink (`/var`), so a + /// root compared by what it canonicalizes to would refuse every + /// absolute path spelled the way the caller got it. + #[cfg(unix)] + #[tokio::test] + async fn the_root_is_matched_as_the_caller_spells_it() { + let (dir, _) = root(); + std::os::unix::fs::symlink(dir.path().join("project"), dir.path().join("alias")) + .expect("symlink"); + let spelled = dir.path().join("alias"); + let fs = Confined::new(&spelled).expect("absolute"); + assert_eq!( + fs.read(&spelled.join("icp.yaml")).await.unwrap(), + b"canisters: []" + ); + assert_eq!( + fs.read(Path::new("icp.yaml")).await.unwrap(), + b"canisters: []" + ); + assert!(fs.is_dir(&spelled).await); + } + + #[test] + fn a_root_that_is_not_absolute_is_refused() { + let err = Confined::new(Path::new("project")).unwrap_err(); + assert!(err.to_string().contains("not an absolute path"), "{err}"); + } + + /// A path through a symlink is refused wherever the link leads: out + /// of the root, back inside it, or nowhere. A link in the middle of + /// a path counts as much as one at its end. + #[cfg(unix)] + #[tokio::test] + async fn a_path_through_a_symlink_is_refused_wherever_it_leads() { + use std::os::unix::fs::symlink; + let (dir, fs) = root(); + let project = fs.root().to_path_buf(); + symlink(dir.path().join("outside.txt"), project.join("out.txt")).expect("symlink"); + symlink(project.join("icp.yaml"), project.join("in.yaml")).expect("symlink"); + symlink(project.join("nowhere"), project.join("dangling")).expect("symlink"); + symlink(project.join("canisters"), project.join("dir-link")).expect("symlink"); + + for (spelling, link) in [ + ("out.txt", "out.txt"), + ("in.yaml", "in.yaml"), + ("dangling", "dangling"), + ("dir-link/a.wasm", "dir-link"), + ("./dir-link/../dir-link/a.wasm", "dir-link"), + // The host would take `..` from where the link leads; the + // spelling alone would cancel the link out and find the file. + ("dir-link/../canisters/a.wasm", "dir-link"), + ("dangling/../icp.yaml", "dangling"), + ] { + let err = fs + .read(Path::new(spelling)) + .await + .expect_err(spelling) + .to_string(); + assert!(err.contains("passes through"), "{spelling}: {err}"); + assert!(err.contains(link), "{spelling}: {err}"); + assert!(!fs.exists(Path::new(spelling)).await, "{spelling}"); + assert!(!fs.is_file(Path::new(spelling)).await, "{spelling}"); + assert!(!fs.is_dir(Path::new(spelling)).await, "{spelling}"); + assert!( + fs.canonicalize(Path::new(spelling)).await.is_none(), + "{spelling}" + ); + } + // Nor may a link be written through. + let err = fs + .write(Path::new("out.txt"), b"x") + .await + .unwrap_err() + .to_string(); + assert!(err.contains("passes through"), "{err}"); + assert_eq!( + std::fs::read(dir.path().join("outside.txt")).unwrap(), + b"secret" + ); + } + + #[tokio::test] + async fn scratch_is_made_under_the_root() { + let (_dir, fs) = root(); + let scratch = fs.scratch_dir().await.expect("scratch"); + assert!(scratch.path().starts_with(fs.root())); + let out = scratch.path().join("out/module.wasm"); + fs.create_dir_all(&scratch.path().join("out")) + .await + .expect("mkdir"); + fs.write(&out, b"built").await.expect("write"); + assert_eq!(fs.read(&out).await.unwrap(), b"built"); + let path = scratch.path().to_path_buf(); + drop(scratch); + assert!(!path.exists()); + } + } + /// Builds a tree of empty files, creating parents as needed, and returns its /// root. fn tree(paths: &[&str]) -> camino_tempfile::Utf8TempDir { diff --git a/crates/icp-project/src/project.rs b/crates/icp-project/src/project.rs index e8f28cfbc..83eb475f7 100644 --- a/crates/icp-project/src/project.rs +++ b/crates/icp-project/src/project.rs @@ -1690,6 +1690,115 @@ pub async fn consolidate_manifest( }) } +/// A project loaded through a [`Confined`] filesystem reads nothing outside +/// its root, whatever path its manifest names: the loader resolves every +/// path it reads itself through the filesystem it is given, so the one rule +/// holds them all. (A local recipe template is read by the [`Resolve`] +/// implementation instead, which is why these tests supply none.) +#[cfg(all(test, feature = "host"))] +mod confined_tests { + use super::*; + use crate::canister::recipe::{Fetched, Resolve, ResolveError}; + use crate::files::{Confined, HostFileSystem}; + use crate::manifest::recipe::Recipe; + use camino_tempfile::Utf8TempDir; + + struct NoRecipes; + + #[async_trait::async_trait] + impl Resolve for NoRecipes { + async fn resolve(&self, _recipe: &Recipe) -> Result { + unreachable!("no manifest here names a recipe") + } + } + + /// A tree with a project directory and, beside it, what the project must + /// not reach: an argument file, and a canister directory of its own. + fn tree(manifest: &str) -> Utf8TempDir { + let dir = Utf8TempDir::new().expect("temp dir"); + let project = dir.path().join("project"); + std::fs::create_dir_all(&project).expect("mkdir"); + std::fs::write(project.join(PROJECT_MANIFEST), manifest).expect("write"); + std::fs::write(dir.path().join("outside.bin"), b"secret").expect("write"); + std::fs::create_dir_all(dir.path().join("elsewhere")).expect("mkdir"); + std::fs::write( + dir.path().join("elsewhere").join(CANISTER_MANIFEST), + "name: stray\nbuild:\n steps:\n - type: pre-built\n path: stray.wasm\n", + ) + .expect("write"); + dir + } + + async fn load(files: &dyn FileSystem, pdir: &Path) -> Result { + let m: ProjectManifest = load_manifest_from_path(files, &pdir.join(PROJECT_MANIFEST)) + .await + .map_err(|e| chain(&e))?; + consolidate_manifest(files, pdir, &NoRecipes, &m) + .await + .map_err(|e| chain(&e)) + } + + fn chain(e: &(dyn std::error::Error + 'static)) -> String { + let mut text = e.to_string(); + let mut source = e.source(); + while let Some(cause) = source { + text.push_str(": "); + text.push_str(&cause.to_string()); + source = cause.source(); + } + text + } + + #[tokio::test] + async fn an_argument_file_outside_the_root_is_read_by_the_host_and_refused_confined() { + let dir = tree(indoc::indoc! {" + canisters: + - name: backend + init_args: + path: ../outside.bin + format: bin + build: + steps: + - type: pre-built + path: backend.wasm + "}); + let pdir = dir.path().join("project"); + + let project = load(&HostFileSystem, &pdir) + .await + .expect("the host reads anywhere"); + let (_, backend) = &project.canisters["backend"]; + assert_eq!( + backend.init_args, + Some(CanisterArgs::Binary(b"secret".to_vec())) + ); + + let confined = Confined::new(&pdir).expect("absolute"); + let err = load(&confined, &pdir) + .await + .expect_err("confined, the read is refused"); + assert!(err.contains("init_args"), "{err}"); + assert!(err.contains("is outside"), "{err}"); + } + + #[tokio::test] + async fn a_canister_directory_outside_the_root_is_found_by_the_host_and_not_confined() { + let dir = tree("canisters:\n - ../elsewhere\n"); + let pdir = dir.path().join("project"); + + let project = load(&HostFileSystem, &pdir) + .await + .expect("the host reads anywhere"); + assert!(project.canisters.contains_key("stray")); + + let confined = Confined::new(&pdir).expect("absolute"); + let err = load(&confined, &pdir) + .await + .expect_err("confined, the directory is not there"); + assert!(err.contains("../elsewhere"), "{err}"); + } +} + #[cfg(all(test, feature = "host"))] mod recipe_sync_tests { use super::*;