From c9d73ece85f636e963612c219e48965e86740d71 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 13:05:47 +0000 Subject: [PATCH] fix(fonts): confine craft-fonts manifest paths and publish licences as text --- crates/text/build.rs | 5 ++++ crates/text/build/web_fonts.rs | 4 ++- xtask/src/web_fonts.rs | 54 ++++++++++++++++++++++++++++------ 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/crates/text/build.rs b/crates/text/build.rs index 25544a3a6..412c471c1 100644 --- a/crates/text/build.rs +++ b/crates/text/build.rs @@ -81,6 +81,7 @@ fn craft_fonts(dir: &Path) -> Result { println!("cargo::rerun-if-changed={}", manifest.display()); let text = std::fs::read_to_string(&manifest).map_err(|e| format!("{}: {e}", manifest.display()))?; let wasm = std::env::var("CARGO_CFG_TARGET_ARCH").is_ok_and(|a| a == "wasm32"); + let root = dir.canonicalize().map_err(|e| format!("{}: {e}", dir.display()))?; let mut out = String::new(); for line in text.lines().map(str::trim).filter(|l| !l.is_empty() && !l.starts_with('#')) { let f: Vec<&str> = line.split(" | ").map(str::trim).collect(); @@ -91,6 +92,10 @@ fn craft_fonts(dir: &Path) -> Result { continue; } let path = dir.join(file).canonicalize().map_err(|e| format!("{file}: {e}"))?; + // The manifest is supplier data: embed only regular files inside the checkout. + if !path.starts_with(&root) || !path.is_file() { + return Err(format!("{file}: not a file inside the font checkout")); + } println!("cargo::rerun-if-changed={}", path.display()); let scripts: Vec = scripts.split(',').map(|s| format!("{:?}", s.trim())).collect(); let _ = writeln!( diff --git a/crates/text/build/web_fonts.rs b/crates/text/build/web_fonts.rs index a2e2a8054..ccd387afa 100644 --- a/crates/text/build/web_fonts.rs +++ b/crates/text/build/web_fonts.rs @@ -69,7 +69,8 @@ pub fn sri(hex: &str) -> Option { /// name>`. Relative, so the site works under any path; content-addressed, so it can be cached /// forever. pub fn url(sha256: &str, file: &str) -> String { - let name = file.rsplit('/').next().unwrap_or(file); + // Either separator: on Windows a backslash in supplier data would otherwise pick a folder. + let name = file.rsplit(['/', '\\']).next().unwrap_or(file); format!("fonts/{}/{name}", sha256.get(..16).unwrap_or(sha256)) } @@ -98,6 +99,7 @@ mod tests { fn url_is_relative_and_content_addressed() { let sha = "63111b5b2e074dd48cc67692e0a2726d86ee94c1c37fe8598257b7b4e87e869e"; assert_eq!(url(sha, "fonts/noto-sans-arabic/NotoSansArabic.ttf"), "fonts/63111b5b2e074dd4/NotoSansArabic.ttf"); + assert_eq!(url(sha, "fonts\\..\\..\\NotoSansArabic.ttf"), "fonts/63111b5b2e074dd4/NotoSansArabic.ttf"); assert!(!url(sha, "x.ttf").starts_with('/')); } diff --git a/xtask/src/web_fonts.rs b/xtask/src/web_fonts.rs index 4399913a8..b5f3e42d2 100644 --- a/xtask/src/web_fonts.rs +++ b/xtask/src/web_fonts.rs @@ -51,7 +51,8 @@ fn copy(list_path: &Path, dir: &Path, site: &Path) -> Result<(usize, Vec let (Some(file), Some(licence), Some(sha)) = (row.get(2), row.get(5), row.get(6)) else { return Err(format!("malformed manifest row for {} {}", e.family, e.style)); }; - let bytes = std::fs::read(dir.join(file)).map_err(|err| format!("{file}: {err}"))?; + let root = std::fs::canonicalize(dir).map_err(|err| format!("{}: {err}", dir.display()))?; + let bytes = std::fs::read(inside(&root, file)?).map_err(|err| format!("{file}: {err}"))?; let got = sha256::hex(&bytes); if got != *sha { return Err(format!("{file}: sha256 {got}, manifest says {sha}")); @@ -60,19 +61,29 @@ fn copy(list_path: &Path, dir: &Path, site: &Path) -> Result<(usize, Vec let folder = dest.parent().ok_or_else(|| format!("{}: no parent folder", dest.display()))?; std::fs::create_dir_all(folder).map_err(|err| format!("{}: {err}", folder.display()))?; std::fs::write(&dest, &bytes).map_err(|err| format!("{}: {err}", dest.display()))?; - let licence_name = Path::new(licence).file_name().ok_or_else(|| format!("{licence}: not a file"))?; - // The manifest is supplier data: the licence must be a file inside the checkout. - let root = std::fs::canonicalize(dir).map_err(|err| format!("{}: {err}", dir.display()))?; - let src = std::fs::canonicalize(dir.join(licence)).map_err(|err| format!("{licence}: {err}"))?; - if !src.starts_with(&root) || !src.is_file() { - return Err(format!("{licence}: not a file inside the font checkout")); - } - std::fs::copy(&src, folder.join(licence_name)).map_err(|err| format!("{licence}: {err}"))?; + let src = inside(&root, licence)?; + // Served as plain text whatever the supplier named it: a `.html` licence would be a page. + let stem = src.file_stem().ok_or_else(|| format!("{licence}: not a file"))?.to_string_lossy(); + std::fs::copy(&src, folder.join(format!("{stem}.txt"))).map_err(|err| format!("{licence}: {err}"))?; copied += 1; } Ok((copied, skipped)) } +/// The biggest font or licence file copied (bytes). +const MAX_FILE: u64 = 64 << 20; + +/// Manifest path `file` (supplier data) as a regular file of at most [`MAX_FILE`] bytes inside the +/// canonical checkout `root`: no `..`, absolute path or symlink out of it, and no device or FIFO. +fn inside(root: &Path, file: &str) -> Result { + let src = std::fs::canonicalize(root.join(file)).map_err(|err| format!("{file}: {err}"))?; + let meta = std::fs::metadata(&src).map_err(|err| format!("{file}: {err}"))?; + if !src.starts_with(root) || !meta.is_file() || meta.len() > MAX_FILE { + return Err(format!("{file}: not a file of at most {MAX_FILE} bytes inside the font checkout")); + } + Ok(src) +} + #[cfg(test)] mod tests { use super::*; @@ -127,6 +138,31 @@ mod tests { assert!(e.contains("inside the font checkout"), "{e}"); } + #[test] + fn a_font_path_that_leaves_the_checkout_or_is_no_file_is_refused() { + let (list, dir, site, _) = fixture("font-escape", None); + let m = dir.join("fonts/manifest.txt"); + let manifest = std::fs::read_to_string(&m).unwrap(); + let outside = dir.parent().unwrap().join("secret.ttf"); + std::fs::write(&outside, b"secret").unwrap(); + for path in ["../secret.ttf", outside.to_str().unwrap(), "fonts/a"] { + std::fs::write(&m, manifest.replace("fonts/a/A-Regular.ttf", path)).unwrap(); + let e = copy(&list, &dir, &site).unwrap_err(); + assert!(e.contains("inside the font checkout"), "{path}: {e}"); + } + } + + #[test] + fn a_licence_is_published_as_text() { + let (list, dir, site, sha) = fixture("html-licence", None); + std::fs::write(dir.join("fonts/a/LICENSE.html"), b"