fix: refuse FIFOs and devices before opening them; bound effect text-file reads

crates/effects/src/mocha_shape.rs

@@ -108,8 +108,12 @@ fn load(src: &str) -> Option<Arc<Vec<MochaShape>>> {
     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();

crates/effects/src/ocio_config.rs

@@ -1321,7 +1321,7 @@ pub fn load_config(src: &str) -> Result<Arc<Config>, 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() {

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<Arc<Lut>> {
         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<String> {
+    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();
+    }
+}

crates/engine/src/analysis/models.rs

@@ -619,7 +619,9 @@ fn install(s: &mut Session, p: &Value, task: Task) -> Result<Value> {
     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)?,

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<Vec<u8>> {
         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() {

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<Vec<u8>> {
     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();
+    }
+}

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<Self> {
+        // 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() {