security(plugins): reject host routing keys, bound bands and start fuel, open without blocking on FIFOs

Cargo.lock

@@ -4963,6 +4963,7 @@ dependencies = [
  "photocraft-geom",
  "photocraft-raster",
  "rayon",
+ "rustix 1.1.5",
  "serde",
  "serde_json",
  "thiserror 2.0.21",

crates/plugins/Cargo.toml

@@ -24,6 +24,10 @@ wasmi = { version = "2.0.0", default-features = false, features = ["std", "valid
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
 rayon = { workspace = true }
 
+# Opening a plug-in file without blocking on a FIFO (the flag constant only).
+[target.'cfg(unix)'.dependencies]
+rustix = { version = "1.1", default-features = false, features = ["std", "fs"] }
+
 [dev-dependencies]
 # WebAssembly text -> binary for the hostile-module fixtures (Apache-2.0 WITH LLVM-exception).
 wat = "1.228"

crates/plugins/src/apply.rs

@@ -70,6 +70,9 @@ fn read_clamped(s: &Surface, rect: Rect, extent: Rect) -> Vec<f32> {
     out
 }
 
+/// Most bands one run is split into.
+const MAX_BANDS: u64 = 1 << 16;
+
 impl Plugin {
     /// The rectangle a run writes: the layer's content (grown by the plug-in's overlap) or the
     /// canvas, limited to the selection's bounds.
@@ -113,6 +116,13 @@ impl Plugin {
         }
         let mut bands = Vec::new();
         let (t0, t1) = (TileCoord::containing(area.x0, area.y0), TileCoord::containing(area.x1 - 1, area.y1 - 1));
+        // Counted before any is made: a few distant tiles make `area` a grid of millions of
+        // bands, each a call into the plug-in, planned before the deadline starts.
+        let columns = (i64::from(t1.tx) - i64::from(t0.tx) + 1).max(0) as u64;
+        let rows = (i64::from(t1.ty) - i64::from(t0.ty) + 1).max(0) as u64;
+        if columns.div_ceil(across as u64).saturating_mul(rows) > MAX_BANDS {
+            return Err(Error::Limit(format!("more than {MAX_BANDS} bands (the layer's content is spread too widely)")));
+        }
         for ty in t0.ty..=t1.ty {
             let mut tx = t0.tx;
             while tx <= t1.tx {

crates/plugins/src/manifest.rs

@@ -398,8 +398,12 @@ fn visible(c: char) -> bool {
     !c.is_whitespace() && !c.is_control() && !in_ranges(c, FORMAT) && !in_ranges(c, BLANK)
 }
 
+/// Names the host's `plugin.run` command takes for itself: a parameter of the same name would be
+/// read as the command's routing (which layer or channel is filtered), not as the plug-in's.
+const HOST_KEYS: [&str; 4] = ["id", "layer", "target", "channel"];
+
 fn valid_key(k: &str) -> bool {
-    (1..=64).contains(&k.len()) && k.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'_') && !k.starts_with('_') && k != "id" && k != "layer"
+    (1..=64).contains(&k.len()) && k.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'_') && !k.starts_with('_') && !HOST_KEYS.contains(&k)
 }
 
 #[cfg(test)]
@@ -471,6 +475,9 @@ mod tests {
             r#"{"id":"a","name":"x","kind":"filter","params":{"r":{"type":"number","min":5,"max":1,"default":1}}}"#,
             r#"{"id":"a","name":"x","kind":"filter","params":{"c":{"type":"choice","options":[]}}}"#,
             r#"{"id":"a","name":"x","kind":"filter","params":{"__x":{"type":"bool"}}}"#,
+            // Names the host command routes by: a plug-in default must not steer the filter.
+            r#"{"id":"a","name":"x","kind":"filter","params":{"target":{"type":"bool","default":false}}}"#,
+            r#"{"id":"a","name":"x","kind":"filter","params":{"channel":{"type":"int","min":0,"max":3,"default":1}}}"#,
             r#"{"id":"a","name":"x","kind":"filter","params":[1]}"#,
         ] {
             assert!(Manifest::parse(bad.as_bytes()).is_err(), "{bad}");

crates/plugins/src/registry.rs

@@ -54,19 +54,35 @@ pub fn revision() -> u64 {
     REVISION.load(Ordering::Relaxed)
 }
 
+/// Opens `path` for reading without waiting on a FIFO or device that has no writer.
+#[cfg(unix)]
+fn open_regular(path: &std::path::Path) -> std::io::Result<std::fs::File> {
+    use std::os::unix::fs::OpenOptionsExt;
+    // O_NONBLOCK: opening a writer-less FIFO read-only returns at once; the caller then sees it
+    // is not a regular file. Regular files ignore the flag.
+    std::fs::OpenOptions::new().read(true).custom_flags(rustix::fs::OFlags::NONBLOCK.bits() as i32).open(path)
+}
+
+#[cfg(not(unix))]
+fn open_regular(path: &std::path::Path) -> std::io::Result<std::fs::File> {
+    std::fs::File::open(path)
+}
+
 /// Reads a `.wasm` file (refusing anything that isn't a regular file or exceeds the module limit)
 /// and loads it.
 #[cfg(not(target_arch = "wasm32"))]
 pub fn load_file(path: &std::path::Path, limits: Limits) -> Result<Plugin> {
     use std::io::Read;
-    let meta = std::fs::metadata(path).map_err(|e| Error::Io(format!("{}: {e}", path.display())))?;
+    // Open first, without blocking on a FIFO, and judge the file that was opened: a name checked
+    // and then opened again can be swapped for a special file in between.
+    let f = open_regular(path).map_err(|e| Error::Io(format!("{}: {e}", path.display())))?;
+    let meta = f.metadata().map_err(|e| Error::Io(format!("{}: {e}", path.display())))?;
     if !meta.is_file() {
         return Err(Error::Io(format!("{} is not a file", path.display())));
     }
     if meta.len() > limits.max_module_bytes as u64 {
         return Err(Error::Module(format!("{} is {} bytes (limit {})", path.display(), meta.len(), limits.max_module_bytes)));
     }
-    let f = std::fs::File::open(path).map_err(|e| Error::Io(format!("{}: {e}", path.display())))?;
     let mut bytes = Vec::new();
     f.take(limits.max_module_bytes as u64 + 1).read_to_end(&mut bytes).map_err(|e| Error::Io(format!("{}: {e}", path.display())))?;
     Ok(Plugin::load(&bytes, limits)?.with_source(path.display().to_string()))

crates/plugins/src/runtime.rs

@@ -184,8 +184,9 @@ impl Plugin {
         let limits = StoreLimitsBuilder::new().memory_size(self.limits.max_memory_bytes).memories(1).tables(4).table_elements(100_000).instances(1).build();
         let mut store = Store::new(&self.engine, limits);
         store.limiter(|l| l);
-        // The start function (if any) runs on this fuel.
-        store.set_fuel(self.limits.fuel_base).map_err(wasm_err)?;
+        // The start function (if any) runs on this fuel. It cannot be interrupted to check the
+        // deadline the way a call can, so it gets no more than one slice, however much a call may use.
+        store.set_fuel(self.limits.fuel_base.min(FUEL_SLICE)).map_err(wasm_err)?;
         let linker = Linker::<StoreLimits>::new(&self.engine);
         let instance = linker.instantiate_and_start(&mut store, &self.module).map_err(wasm_err)?;
         let memory = instance.get_memory(&store, "memory").ok_or_else(|| Error::Abi("the module must export its linear memory as `memory`".into()))?;

crates/plugins/tests/abi.rs

@@ -317,3 +317,44 @@ fn folder_loading_skips_bad_files() {
     assert!(photocraft_plugins::registry::load_file(&dir, Limits::default()).is_err(), "a directory is not a module");
     std::fs::remove_dir_all(&dir).ok();
 }
+
+#[test]
+fn a_start_function_that_spins_is_cut_off_at_one_slice() {
+    // `fuel_base` is what a call may use; the start function cannot be interrupted to check the
+    // clock, so it gets one slice of it at most.
+    let spin = "(func $spin (loop $l (br $l))) (start $spin)";
+    let limits = Limits { fuel_base: u64::MAX / 4, ..Limits::default() };
+    let (error, took) = run_err(&module(MANIFEST, 1, "(i32.const 0)", spin), limits);
+    assert!(matches!(error, Error::Trap(_) | Error::Limit(_)), "{error:?}");
+    assert!(took < Duration::from_secs(30), "{took:?}");
+}
+
+#[test]
+fn content_spread_over_millions_of_bands_is_refused_before_planning_them() {
+    let p = Plugin::load(INVERT, Limits::default()).unwrap();
+    let mut s = Surface::new(PixelFormat::RGBA8);
+    s.write_pixel(0, 0, &[0.1, 0.2, 0.3, 1.0]);
+    s.write_pixel(4_000_000, 4_000_000, &[0.1, 0.2, 0.3, 1.0]);
+    let t = Instant::now();
+    let e = p.apply(&s, Rect::new(0, 0, 8, 8), None, &json!({})).expect_err("a 15600 × 15600 tile grid is not one run");
+    assert!(matches!(e, Error::Limit(_)), "{e:?}");
+    assert!(t.elapsed() < Duration::from_secs(5), "{:?}", t.elapsed());
+}
+
+#[cfg(unix)]
+#[test]
+fn a_fifo_in_place_of_a_module_file_does_not_block_loading() {
+    let dir = std::env::temp_dir().join(format!("photocraft-plugin-fifo-{}", std::process::id()));
+    std::fs::create_dir_all(&dir).unwrap();
+    let path = dir.join("evil.wasm");
+    let _ = std::fs::remove_file(&path);
+    rustix::fs::mknodat(rustix::fs::CWD, &path, rustix::fs::FileType::Fifo, rustix::fs::Mode::RUSR | rustix::fs::Mode::WUSR, 0).unwrap();
+    let (tx, rx) = std::sync::mpsc::channel();
+    let p = path.clone();
+    std::thread::spawn(move || {
+        let _ = tx.send(photocraft_plugins::registry::load_file(&p, Limits::default()).map(|_| ()));
+    });
+    let r = rx.recv_timeout(Duration::from_secs(10)).expect("loading a FIFO must not block");
+    assert!(matches!(r, Err(Error::Io(_))), "{r:?}");
+    let _ = std::fs::remove_dir_all(&dir);
+}