diff --git a/crates/engine/src/services.rs b/crates/engine/src/services.rs index adbd3ab81..2d425a4e6 100644 --- a/crates/engine/src/services.rs +++ b/crates/engine/src/services.rs @@ -50,8 +50,20 @@ pub trait Services: Send + Sync { pub struct FsServices; impl Services for FsServices { + /// Refuses what isn't a regular file: a project can name `/dev/zero` or a FIFO, and reading + /// those never ends (#2134). fn read_file(&self, path: &str) -> std::io::Result> { - std::fs::read(path) + use std::io::Read; + let mut file = std::fs::File::open(path)?; + let meta = file.metadata()?; + if !meta.is_file() { + return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{path} is not a file"))); + } + let mut bytes = Vec::new(); + // A sparse file can claim far more than memory: an error, not an abort. + bytes.try_reserve_exact(usize::try_from(meta.len()).unwrap_or(usize::MAX)).map_err(|e| std::io::Error::new(std::io::ErrorKind::OutOfMemory, e))?; + file.read_to_end(&mut bytes)?; + Ok(bytes) } fn write_file(&self, path: &str, data: &[u8]) -> std::io::Result<()> { config::atomic_write(std::path::Path::new(path), data) @@ -70,3 +82,21 @@ pub trait Importer: Send + Sync { /// media layer reads from memory (the browser) register them; file-based ones need nothing. fn add_frames(&self, _paths: &[String]) {} } + +#[cfg(test)] +mod tests { + use super::{FsServices, Services}; + + /// #2134: a project can name a folder, `/dev/zero` or a FIFO; reading those is an error. + #[test] + fn fs_services_read_only_regular_files() { + let dir = std::env::temp_dir(); + assert!(FsServices.read_file(&dir.to_string_lossy()).is_err()); + #[cfg(unix)] + assert!(FsServices.read_file("/dev/zero").is_err()); + let f = dir.join(format!("ec-services-read-{}.txt", std::process::id())); + std::fs::write(&f, b"data").unwrap(); + assert_eq!(FsServices.read_file(&f.to_string_lossy()).unwrap(), b"data"); + let _ = std::fs::remove_file(f); + } +} diff --git a/crates/media/src/lib.rs b/crates/media/src/lib.rs index 9b5b491ab..306f8d860 100644 --- a/crates/media/src/lib.rs +++ b/crates/media/src/lib.rs @@ -79,6 +79,22 @@ pub fn is_importable(path: &std::path::Path) -> bool { || dpx::EXTENSIONS.contains(&ext.as_str()) } +/// A whole file's bytes, refusing anything that isn't a regular file: a project can name +/// `/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; + let mut file = std::fs::File::open(path)?; + let meta = file.metadata()?; + if !meta.is_file() { + return Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{} is not a file", path.display()))); + } + let mut bytes = Vec::new(); + // A sparse file can claim far more than memory: an error, not an abort. + bytes.try_reserve_exact(usize::try_from(meta.len()).unwrap_or(usize::MAX)).map_err(|e| std::io::Error::new(std::io::ErrorKind::OutOfMemory, e))?; + file.read_to_end(&mut bytes)?; + Ok(bytes) +} + pub(crate) fn ext_of(path: &std::path::Path) -> String { path.extension().and_then(std::ffi::OsStr::to_str).unwrap_or("").to_ascii_lowercase() } @@ -99,3 +115,20 @@ pub(crate) fn still_format(path: &str, bytes: &[u8]) -> Option effectcraft_time::FrameRate { if r.num > 0 && r.den > 0 { effectcraft_time::FrameRate::new(r.num, r.den) } else { effectcraft_time::FrameRate::FPS_30 } } + +#[cfg(test)] +mod read_tests { + /// #2134: a folder (and on Unix a device) named as footage is an error, not an endless read. + #[test] + fn only_regular_files_are_read_whole() { + let dir = std::env::temp_dir(); + assert!(super::read_regular_file(&dir).is_err()); + assert!(crate::probe::probe(&dir).is_err(), "probing a folder fails instead of reading it"); + #[cfg(unix)] + assert!(super::read_regular_file(std::path::Path::new("/dev/zero")).is_err()); + let f = dir.join(format!("ec-read-regular-{}.bin", std::process::id())); + std::fs::write(&f, b"abc").unwrap(); + assert_eq!(super::read_regular_file(&f).unwrap(), b"abc"); + let _ = std::fs::remove_file(f); + } +} diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index 748d33c99..6e0c9043b 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -581,7 +581,7 @@ impl Inner { if let Some(b) = lock(&self.files).get(path) { return Ok(b.clone()); } - std::fs::read(path).map(Into::into).map_err(|e| MediaError::Io(format!("{path}: {e}"))) + crate::read_regular_file(std::path::Path::new(path)).map(Into::into).map_err(|e| MediaError::Io(format!("{path}: {e}"))) } /// The opened movie/audio source for `path`. The first caller opens it, outside the map's diff --git a/crates/media/src/probe.rs b/crates/media/src/probe.rs index 6bba682b3..8197cd61f 100644 --- a/crates/media/src/probe.rs +++ b/crates/media/src/probe.rs @@ -25,7 +25,7 @@ pub fn probe(path: impl AsRef) -> Result { correct_rate(&mut f, header.as_deref().and_then(video_rate)); return Ok(f); } - let bytes = std::fs::read(path).map_err(|e| MediaError::Io(format!("{}: {e}", path.display())))?; + let bytes = crate::read_regular_file(path).map_err(|e| MediaError::Io(format!("{}: {e}", path.display())))?; if crate::MODEL_EXTENSIONS.contains(&ext_of(path).as_str()) { return probe_model(&path.to_string_lossy(), &bytes, &side_file_reader(path)); } @@ -38,7 +38,7 @@ fn side_file_reader(model: &Path) -> impl Fn(&str) -> Option> + use<> { let model = std::path::absolute(model).unwrap_or_else(|_| model.to_path_buf()); move |uri| { let p = craft_docpath::resolve_absolute(uri, Some(&model)).ok()?; - std::fs::read(p).ok() + crate::read_regular_file(&p).ok() } }