diff --git a/crates/effects/src/mocha_shape.rs b/crates/effects/src/mocha_shape.rs index cf9e2bd88..27937873a 100644 --- a/crates/effects/src/mocha_shape.rs +++ b/crates/effects/src/mocha_shape.rs @@ -108,8 +108,12 @@ fn load(src: &str) -> Option>> { if let Some(v) = cache.lock().ok().and_then(|c| c.get(src).cloned()) { return v; } - let parsed = - if s.starts_with('{') || s.starts_with('[') { parse_shapes(s) } else { std::fs::read_to_string(s).ok().and_then(|t| parse_shapes(&t)) }.map(Arc::new); + let parsed = if s.starts_with('{') || s.starts_with('[') { + parse_shapes(s) + } else { + crate::utility::read_text_file(std::path::Path::new(s)).ok().and_then(|t| parse_shapes(&t)) + } + .map(Arc::new); if let Ok(mut c) = cache.lock() { if c.len() > 64 { c.clear(); diff --git a/crates/effects/src/ocio_config.rs b/crates/effects/src/ocio_config.rs index af860c2fc..674b0abc5 100644 --- a/crates/effects/src/ocio_config.rs +++ b/crates/effects/src/ocio_config.rs @@ -1321,7 +1321,7 @@ pub fn load_config(src: &str) -> Result, String> { Config::parse(src, Path::new(".")) } else { let p = Path::new(t); - std::fs::read_to_string(p).map_err(|e| format!("{t}: {e}")).and_then(|text| Config::parse(&text, p.parent().unwrap_or(Path::new(".")))) + crate::utility::read_text_file(p).map_err(|e| format!("{t}: {e}")).and_then(|text| Config::parse(&text, p.parent().unwrap_or(Path::new(".")))) } .map(Arc::new); if let Ok(mut m) = cache.lock() { diff --git a/crates/effects/src/utility.rs b/crates/effects/src/utility.rs index e41b42e99..e7460be62 100644 --- a/crates/effects/src/utility.rs +++ b/crates/effects/src/utility.rs @@ -2,6 +2,7 @@ //! lookup tables (.cube). use std::collections::HashMap; +use std::path::Path; use std::sync::{Arc, Mutex, OnceLock}; use effectcraft_color::luminance; @@ -332,13 +333,32 @@ pub fn load_lut(src: &str) -> Option> { return v.clone(); } let inline = src.contains('\n') || src.contains("LUT_"); - let parsed = if inline { parse_cube(src) } else { std::fs::read_to_string(src.trim()).ok().and_then(|t| parse_cube(&t)) }.map(Arc::new); + let parsed = if inline { parse_cube(src) } else { read_text_file(Path::new(src.trim())).ok().and_then(|t| parse_cube(&t)) }.map(Arc::new); if let Ok(mut m) = cache.lock() { m.insert(src.to_string(), parsed.clone()); } parsed } +/// Largest text file an effect reads (a LUT, an OCIO config, Mocha shape data). +const MAX_TEXT_FILE_BYTES: u64 = 256 << 20; + +/// A text file an effect parameter or a saved project names: only a regular file, and only up to +/// [`MAX_TEXT_FILE_BYTES`] (`/dev/zero` or a FIFO would never end). +pub(crate) fn read_text_file(path: &Path) -> std::io::Result { + use std::io::Read; + let meta = std::fs::metadata(path)?; + if !meta.is_file() || meta.len() > MAX_TEXT_FILE_BYTES { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("{} is not a text file of at most {} MiB", path.display(), MAX_TEXT_FILE_BYTES >> 20), + )); + } + let mut text = String::new(); + std::fs::File::open(path)?.take(MAX_TEXT_FILE_BYTES).read_to_string(&mut text)?; + Ok(text) +} + fn apply_lut(ctx: &EffectCtx, mut b: Buf) -> Buf { let Some(lut) = load_lut(ctx.params.s("lut")) else { return b; @@ -588,3 +608,19 @@ mod tests { assert_eq!(out.offset, [5.0, 5.0]); } } + +#[cfg(all(test, unix))] +mod read_text_file_tests { + #[test] + fn devices_and_fifos_are_refused_without_reading() { + assert!(super::read_text_file(std::path::Path::new("/dev/zero")).is_err()); + let dir = std::env::temp_dir().join(format!("ec-fx-fifo-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let fifo = dir.join("lut.cube"); + let _ = std::fs::remove_file(&fifo); + assert!(std::process::Command::new("mkfifo").arg(&fifo).status().unwrap().success()); + // Opening a FIFO with no writer would block: this returns at once. + assert!(super::read_text_file(&fifo).is_err()); + std::fs::remove_dir_all(&dir).unwrap(); + } +} diff --git a/crates/engine/src/analysis/models.rs b/crates/engine/src/analysis/models.rs index 63550545a..8e0d78d55 100644 --- a/crates/engine/src/analysis/models.rs +++ b/crates/engine/src/analysis/models.rs @@ -619,7 +619,9 @@ fn install(s: &mut Session, p: &Value, task: Task) -> Result { let cmd = format!("{}.model.install", kind(task).prefix); let path = str_p(p, "path").ok_or_else(|| bad(&cmd, "missing `path` (the downloaded weights file)"))?; let dir = s.models_dir().ok_or_else(|| bad(&cmd, "no settings folder to install into"))?; - let bytes = std::fs::read(path).map_err(|e| EngineError::Other(format!("cannot read {path}: {e}")))?; + // Through the services' reader, which refuses what isn't a regular file (a device would be + // read to no end). + let bytes = s.services.read_file(path).map_err(|e| EngineError::Other(format!("cannot read {path}: {e}")))?; // Which model it is: the one asked for, or the one whose checksum it has. let m = match str_p(p, "id") { Some(_) => model_p(p, &cmd, task)?, diff --git a/crates/engine/src/services.rs b/crates/engine/src/services.rs index 2d425a4e6..47109b661 100644 --- a/crates/engine/src/services.rs +++ b/crates/engine/src/services.rs @@ -54,6 +54,11 @@ impl Services for FsServices { /// those never ends (#2134). fn read_file(&self, path: &str) -> std::io::Result> { use std::io::Read; + // Checked before opening too: opening a FIFO with no writer blocks, so the check after the + // open would never run. shortcut: a FIFO swapped in between still blocks; O_NONBLOCK would close it. + if !std::fs::metadata(path)?.is_file() { + return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{path} is not a file"))); + } let mut file = std::fs::File::open(path)?; let meta = file.metadata()?; if !meta.is_file() { diff --git a/crates/media/src/lib.rs b/crates/media/src/lib.rs index e985985b7..fa8823114 100644 --- a/crates/media/src/lib.rs +++ b/crates/media/src/lib.rs @@ -84,6 +84,11 @@ pub fn is_importable(path: &std::path::Path) -> bool { /// `/dev/zero` or a FIFO as footage, and reading those never ends (#2134). pub(crate) fn read_regular_file(path: &std::path::Path) -> std::io::Result> { use std::io::Read; + // Checked before opening too: opening a FIFO with no writer blocks, so the check after the + // open would never run. shortcut: a FIFO swapped in between still blocks; O_NONBLOCK would close it. + if !std::fs::metadata(path)?.is_file() { + return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{} is not a file", path.display()))); + } let mut file = std::fs::File::open(path)?; let meta = file.metadata()?; if !meta.is_file() { @@ -133,3 +138,19 @@ mod read_tests { let _ = std::fs::remove_file(f); } } + +#[cfg(all(test, unix))] +mod read_regular_file_tests { + #[test] + fn a_fifo_is_refused_before_it_is_opened() { + let dir = std::env::temp_dir().join(format!("ec-media-fifo-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let fifo = dir.join("clip.png"); + let _ = std::fs::remove_file(&fifo); + assert!(std::process::Command::new("mkfifo").arg(&fifo).status().unwrap().success()); + // With no writer, opening it would block forever. + assert!(super::read_regular_file(&fifo).is_err()); + assert!(super::stream::FileReader::open(&fifo).is_err()); + std::fs::remove_dir_all(&dir).unwrap(); + } +} diff --git a/crates/media/src/stream.rs b/crates/media/src/stream.rs index 80db21fc4..1c8bddb30 100644 --- a/crates/media/src/stream.rs +++ b/crates/media/src/stream.rs @@ -24,6 +24,11 @@ pub(crate) struct FileReader { #[cfg(any(unix, windows))] impl FileReader { pub(crate) fn open(path: &Path) -> std::io::Result { + // Checked before opening too: opening a FIFO with no writer blocks, so the check after the + // open would never run. shortcut: a FIFO swapped in between still blocks; O_NONBLOCK would close it. + if !std::fs::metadata(path)?.is_file() { + return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{} is not a file", path.display()))); + } let file = std::fs::File::open(path)?; let m = file.metadata()?; if !m.is_file() {