Templates: Import Template writes embedded media only into its item folders

Each 'media/<item>/<file>' entry is written to '<template> Media/<item>/<file>'
only when <item> is all digits and <file> is a plain file name (not empty, '.'
or '..', no separators, and on Windows no drive, stream or device name), and
the written path is checked to sit in an item folder of '<template> Media'.
read_stored skips entries with absolute, drive or '..' names.
This commit is contained in:
aiongg
2026-10-16 16:14:02 +00:00
parent 8f02f52a44
commit 299223f112
4 changed files with 88 additions and 7 deletions
+17 -2
View File
@@ -891,6 +891,22 @@ pub fn read_template(bytes: &[u8]) -> std::result::Result<(Value, Project, Vec<(
Ok((manifest, project, entries))
}
/// Where a template's `media/<item>/<file>` entry is written: `<media_dir>/<item>/<file>`.
/// `None` unless `<item>` is all digits and `<file>` is a plain file name (not empty, `.` or
/// `..`, no separators, and on Windows no drive, stream or device name), so every entry lands
/// in an item folder directly inside `media_dir`.
fn template_media_path(media_dir: &std::path::Path, entry: &str) -> Option<std::path::PathBuf> {
let (item, file) = entry.strip_prefix("media/")?.split_once('/')?;
if item.is_empty() || !item.bytes().all(|b| b.is_ascii_digit()) {
return None;
}
if !super::scripts::is_bare_name(file) || (cfg!(windows) && super::scripts::windows_unsafe_name(file)) {
return None;
}
let out = media_dir.join(item).join(file);
(out.parent().and_then(std::path::Path::parent) == Some(media_dir) && out.file_name() == Some(std::ffi::OsStr::new(file))).then_some(out)
}
fn import_template(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "essential.importTemplate";
let path = str_p(p, "path").ok_or_else(|| bad(C, "missing `path` (.ectemplate)"))?.to_string();
@@ -905,8 +921,7 @@ fn import_template(s: &mut Session, p: &Value) -> Result<Value> {
let media_dir = dir.join(format!("{stem} Media"));
let mut written = std::collections::HashMap::new();
for (k, v) in entries.iter().filter(|(k, _)| k.starts_with("media/")) {
let rel = k.trim_start_matches("media/");
let out = media_dir.join(rel);
let Some(out) = template_media_path(&media_dir, k) else { continue };
if let Some(parent) = out.parent() {
let _ = std::fs::create_dir_all(parent);
}
+2 -2
View File
@@ -132,7 +132,7 @@ fn is_script_name(name: &str) -> bool {
}
/// One path component on every platform: not empty, `.` or `..`, and without `/`, `\` or NUL.
fn is_bare_name(name: &str) -> bool {
pub(crate) fn is_bare_name(name: &str) -> bool {
!name.is_empty() && name != "." && name != ".." && !name.contains(['/', '\\', '\0'])
}
@@ -140,7 +140,7 @@ fn is_bare_name(name: &str) -> bool {
/// stream), ending in `.` or a space (Windows drops them), or a device name, which names a device
/// in any folder and with any extension (`con.jsx`): `CON`, `PRN`, `AUX`, `NUL`, `CONIN$`,
/// `CONOUT$`, `COM0`–`COM9` and `LPT0`–`LPT9` (also with `¹²³`).
fn windows_unsafe_name(name: &str) -> bool {
pub(crate) fn windows_unsafe_name(name: &str) -> bool {
let stem = name.split('.').next().unwrap_or_default().trim_end_matches(' ').to_ascii_uppercase();
let port = |p: &str| {
let mut n = stem.strip_prefix(p).unwrap_or_default().chars();
+49
View File
@@ -274,6 +274,55 @@ fn template_export_import_round_trip_with_overrides() {
assert!(t.execute("essential.importTemplate", json!({"path": dir.join("bad.ectemplate").to_string_lossy()})).is_err());
}
#[test]
fn template_import_writes_media_only_into_its_media_folder() {
let root = out_dir("ectemplate-names");
let dir = root.join("a").join("b");
std::fs::create_dir_all(&dir).unwrap();
let (mut s, card, _solid, _ids) = card();
let src = dir.join("Src.ectemplate");
s.execute("essential.exportTemplate", json!({"comp": card.0, "path": src.to_string_lossy()})).unwrap();
let mut entries = effectcraft_lottie::zip::read_stored(&std::fs::read(&src).unwrap());
std::fs::remove_file(&src).unwrap();
let abs = root.join("abs").join("x").to_string_lossy().replace('\\', "/");
for name in [
"../x",
"media/../../x",
"media/1/../../../x",
&format!("media/{abs}"),
&format!("media/6{abs}"),
"media/C:\\x",
"media/7/C:\\x",
"media/x/clip.png",
"media/8/sub/clip.png",
"media/2/..",
"media/3/",
"media/4/clip.png",
"media/5/clip.png",
] {
entries.push((name.to_string(), b"data".to_vec()));
}
let path = dir.join("T.ectemplate");
std::fs::write(&path, effectcraft_lottie::zip::store(&entries)).unwrap();
s.execute("essential.importTemplate", json!({"path": path.to_string_lossy()})).unwrap();
let media = dir.join("T Media");
fn files(d: &std::path::Path, out: &mut Vec<std::path::PathBuf>) {
for e in std::fs::read_dir(d).unwrap().map(|e| e.unwrap().path()) {
if e.is_dir() { files(&e, out) } else { out.push(e) }
}
}
let mut found = vec![];
files(&root, &mut found);
let in_item = |f: &std::path::Path| f.parent().and_then(std::path::Path::parent) == Some(media.as_path());
for f in &found {
assert!(f == &path || in_item(f), "written outside an item folder of the media folder: {f:?}");
}
let mut names: Vec<String> = found.iter().filter(|f| in_item(f)).map(|f| f.strip_prefix(&media).unwrap().to_string_lossy().replace('\\', "/")).collect();
names.sort();
assert_eq!(names, ["4/clip.png", "5/clip.png"]);
}
#[test]
fn media_replacement_swaps_footage_per_instance() {
use std::sync::Arc;
+20 -3
View File
@@ -70,8 +70,16 @@ pub fn store(entries: &[(String, Vec<u8>)]) -> Vec<u8> {
out
}
/// Whether an entry name is a relative path inside the archive: not absolute (`/…`, `\…`), no
/// drive (`C:…`) and no `..` component.
fn is_relative_name(name: &str) -> bool {
let drive = name.as_bytes().get(1) == Some(&b':');
!name.starts_with(['/', '\\']) && !drive && !name.split(['/', '\\']).any(|c| c == "..")
}
/// Read the stored (uncompressed) entries of a ZIP archive written by [`store`] or any other
/// writer that did not compress. Compressed entries are skipped.
/// writer that did not compress. Compressed entries and entries whose names are not relative
/// paths inside the archive are skipped.
pub fn read_stored(data: &[u8]) -> Vec<(String, Vec<u8>)> {
let mut out = vec![];
let mut i = 0usize;
@@ -82,8 +90,9 @@ pub fn read_stored(data: &[u8]) -> Vec<(String, Vec<u8>)> {
let name_start = i + 30;
let body = name_start + nlen + xlen;
let (Some(name), Some(bytes)) = (data.get(name_start..name_start + nlen), data.get(body..body + size)) else { break };
if method == 0 {
out.push((String::from_utf8_lossy(name).to_string(), bytes.to_vec()));
let name = String::from_utf8_lossy(name).to_string();
if method == 0 && is_relative_name(&name) {
out.push((name, bytes.to_vec()));
}
i = body + size;
}
@@ -101,4 +110,12 @@ mod tests {
assert_eq!(back[1].0, "b/c.txt");
assert_eq!(back[1].1, b"hello");
}
#[test]
fn entries_outside_the_archive_root_are_skipped() {
let names = ["../up.txt", "a/../../up.txt", "/abs/x", "\\abs\\x", "C:\\x", "c:x", "a\\..\\x", "ok/x.txt", "ok/..x"];
let z = super::store(&names.iter().map(|n| (n.to_string(), b"x".to_vec())).collect::<Vec<_>>());
let back: Vec<String> = super::read_stored(&z).into_iter().map(|(n, _)| n).collect();
assert_eq!(back, ["ok/x.txt", "ok/..x"]);
}
}