* Security: create the --root staging file with create_new Follow-up to #164. write_atomic staged every save at the fixed name .<name>.pdfcraft-tmp and wrote it with fs::write, which follows a symbolic link and writes through a hard link. So a link planted there, in a folder someone else can write to, redirected the bytes to any file the link pointed at. The staging file now gets a random 64-bit suffix (std's RandomState, keyed from the operating system's random source) and is opened with OpenOptions::create_new, which fails if anything already has the name (a file, a hard link, a symbolic link even when dangling), on Windows (CREATE_NEW) as on Unix (O_EXCL). A taken name is skipped, never opened; Windows reports a folder at the name as "access denied", which counts as taken when something is there. After 16 taken names the save fails and nothing is overwritten. A failed write or rename removes only the file this save created. At most 128 bytes of the target's name go into the staging name, so it fits every system's name limit. The #164 tests that planted a sentinel at the old fixed name now also compare folder listings, and say that the "is a folder" refusal is what they check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Security: create the desktop app's save staging file with create_new The same fix as the previous commit, for the desktop app. editing::write_atomically, which Save, Save As, Export, Print to PDF, signing and crash recovery all use, staged at .<name>.pdfcraft-<pid>.tmp and opened it with File::create, which follows a symbolic link and writes through a hard link. A process id is easy to guess, so a link planted beside a document in a shared folder could redirect a save into another file. The staging file now gets a random 64-bit suffix and is opened with OpenOptions::create_new; a taken name (on Windows, a folder reports "access denied") is skipped, never opened, and after 16 the save fails without overwriting anything. At most 128 bytes of the document's name go into the staging name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Brandon Thomas <echelon@gmail.com>
100 lines
4.8 KiB
Rust
100 lines
4.8 KiB
Rust
//! `pdfcraft-cli run --root`: a script's image output stays inside the root (#137).
|
|
|
|
use std::path::{Path, PathBuf};
|
|
use std::process::{Command, Output};
|
|
|
|
const PNG: &[u8] = b"\x89PNG\r\n\x1a\n";
|
|
|
|
/// `root/` next to `outside/` (holding `existing.txt`) in a fresh temporary directory.
|
|
fn sandbox(test: &str) -> PathBuf {
|
|
let base = std::env::temp_dir().join(format!("pdfcraft-cli-{test}-{}", std::process::id()));
|
|
let _ = std::fs::remove_dir_all(&base);
|
|
std::fs::create_dir_all(base.join("root")).unwrap();
|
|
std::fs::create_dir_all(base.join("outside")).unwrap();
|
|
std::fs::write(base.join("outside/existing.txt"), "SENTINEL\n").unwrap();
|
|
base
|
|
}
|
|
|
|
/// Run a two-step script (a blank page, then render it to `out`) from the working directory
|
|
/// `cwd`, with `--root root` when `root` is given.
|
|
fn render_to(base: &Path, cwd: &Path, root: Option<&Path>, out: &str) -> Output {
|
|
let script = base.join("steps.json");
|
|
let steps = serde_json::json!([
|
|
{ "tool": "doc_create", "args": { "from": "blank", "width": 100, "height": 100 } },
|
|
{ "tool": "page_render", "args": { "doc": 1, "page": 1, "dpi": 10 }, "out": out },
|
|
]);
|
|
std::fs::write(&script, steps.to_string()).unwrap();
|
|
let mut cmd = Command::new(env!("CARGO_BIN_EXE_pdfcraft-cli"));
|
|
cmd.current_dir(cwd).arg("run").arg("--script").arg(&script);
|
|
if let Some(root) = root {
|
|
cmd.arg("--root").arg(root);
|
|
}
|
|
cmd.output().unwrap()
|
|
}
|
|
|
|
fn is_png(path: &Path) -> bool {
|
|
std::fs::read(path).is_ok_and(|b| b.starts_with(PNG))
|
|
}
|
|
|
|
#[test]
|
|
fn script_image_output_is_confined_to_the_root() {
|
|
let base = sandbox("run-root");
|
|
let root = base.join("root");
|
|
let abs = |rel: &str| base.join(rel).to_str().unwrap().to_owned();
|
|
|
|
#[cfg_attr(not(windows), allow(unused_mut))]
|
|
let mut refused = vec![abs("outside/escaped.png"), abs("outside/existing.txt"), "../outside/escaped.png".into(), abs("outside/new/dir/x.png")];
|
|
#[cfg(windows)]
|
|
{
|
|
refused.push(r"..\outside\escaped.png".into());
|
|
refused.push(r"\\pdfcraft-test.invalid\share\x.png".into());
|
|
if let Some(drive) = (b'D'..=b'Z').rev().map(|d| format!("{}:\\", d as char)).find(|d| !Path::new(d).exists()) {
|
|
refused.push(format!("{drive}x.png"));
|
|
}
|
|
}
|
|
for out in &refused {
|
|
let o = render_to(&base, &base, Some(&root), out);
|
|
let stderr = String::from_utf8_lossy(&o.stderr);
|
|
assert!(!o.status.success(), "{out}: expected a refusal");
|
|
assert!(stderr.contains("step 2 (page_render)") && stderr.contains("is outside the allowed directory"), "{out}: {stderr}");
|
|
}
|
|
assert!(!base.join("outside/escaped.png").exists() && !base.join("outside/new").exists());
|
|
assert_eq!(std::fs::read_to_string(base.join("outside/existing.txt")).unwrap(), "SENTINEL\n");
|
|
|
|
// "." is the root itself: refused, without staging a temporary file beside the root. The
|
|
// file at the old fixed staging name and the listing are canaries; staging names are random.
|
|
std::fs::write(base.join(".root.pdfcraft-tmp"), "SENTINEL\n").unwrap();
|
|
let listing = || {
|
|
let mut names: Vec<_> = std::fs::read_dir(&base).unwrap().map(|e| e.unwrap().file_name()).collect();
|
|
names.sort();
|
|
names
|
|
};
|
|
let before = listing();
|
|
let o = render_to(&base, &base, Some(&root), ".");
|
|
assert!(!o.status.success() && String::from_utf8_lossy(&o.stderr).contains("is a folder"), "{}", String::from_utf8_lossy(&o.stderr));
|
|
assert_eq!(std::fs::read_to_string(base.join(".root.pdfcraft-tmp")).unwrap(), "SENTINEL\n");
|
|
assert_eq!(listing(), before, "nothing was left beside the root");
|
|
|
|
// A relative path resolves inside the root, not in the working directory.
|
|
let o = render_to(&base, &base, Some(&root), "relative.png");
|
|
assert!(o.status.success(), "{}", String::from_utf8_lossy(&o.stderr));
|
|
assert!(is_png(&root.join("relative.png")) && !base.join("relative.png").exists());
|
|
// So does an absolute one inside the root, and a new folder in it.
|
|
for (out, file) in [(abs("root/inside.png"), "inside.png"), ("pages/one.png".into(), "pages/one.png")] {
|
|
let o = render_to(&base, &base, Some(&root), &out);
|
|
assert!(o.status.success(), "{out}: {}", String::from_utf8_lossy(&o.stderr));
|
|
assert!(is_png(&root.join(file)), "{out}");
|
|
}
|
|
|
|
// Without --root, the path is used as given.
|
|
let o = render_to(&base, &base, None, "free.png");
|
|
assert!(o.status.success(), "{}", String::from_utf8_lossy(&o.stderr));
|
|
assert!(is_png(&base.join("free.png")));
|
|
// Written in place, not through a temporary file beside it, so a device still works.
|
|
#[cfg(unix)]
|
|
{
|
|
let o = render_to(&base, &base, None, "/dev/stdout");
|
|
assert!(o.status.success() && o.stdout.windows(PNG.len()).any(|w| w == PNG), "{}", String::from_utf8_lossy(&o.stderr));
|
|
}
|
|
let _ = std::fs::remove_dir_all(&base);
|
|
}
|