diff --git a/crates/engine/src/cmd/plugin_call.rs b/crates/engine/src/cmd/plugin_call.rs index 5003ec48f..b151adcea 100644 --- a/crates/engine/src/cmd/plugin_call.rs +++ b/crates/engine/src/cmd/plugin_call.rs @@ -13,7 +13,9 @@ //! changes preferences, runs a script, an action or a plug-in (a plug-in command, or an effect a //! plug-in adds), opens, closes or switches documents, or undoes or redoes, whatever its manifest //! lists. A command whose params name a file or folder is refused unless [`NOT_FILES`] says that -//! key names part of the document in it. Every other command is refused too unless +//! key names part of the document in it, and so is a call whose arguments carry such a key at any +//! depth ([`names_file`]); a call runs with no file or folder readable or writable +//! ([`execute_confined`]). Every other command is refused too unless //! [`reviewed::ALLOWED`] lists it, so a command added to the registry later is refused until //! someone decides. @@ -180,6 +182,30 @@ pub(crate) fn names_key(text: &str, key: &str) -> bool { }) } +/// Does `args`, the arguments of a call of command `id`, carry a key in [`FILE_KEYS`] at any depth +/// (in objects and in arrays), such as an effect's `textureImage.path`? Only a key [`NOT_FILES`] +/// names for `id`, at the top level, isn't counted. The walk keeps its own stack, so deep +/// arguments can't overflow the thread's. +fn names_file(id: &str, args: &Value) -> bool { + let mut stack = vec![(args, true)]; + while let Some((v, top)) = stack.pop() { + match v { + Value::Array(a) => stack.extend(a.iter().map(|x| (x, false))), + Value::Object(o) => { + for (k, x) in o { + let k = k.as_str(); + if FILE_KEYS.contains(&k) && !(top && NOT_FILES.contains(&(id, k))) { + return true; + } + stack.push((x, false)); + } + } + _ => {} + } + } + false +} + /// Param keys whose value names an effect or a command (`id` is `effect.apply`'s other name for /// `effect`). const PLUGIN_KEYS: &[&str] = &["effect", "effects", "id", "command", "commands"]; @@ -255,11 +281,11 @@ pub(crate) fn call(s: &mut Session, run: &Calls, id: &str, args: &Value) -> std: let Some(spec) = super::find_command(id) else { return Err(CallFailure { code: CALL_UNKNOWN_COMMAND, why: super::params::unknown_command(id).to_string() }); }; - if denied(spec).is_some() || names_plugin(args) { + if denied(spec).is_some() || names_plugin(args) || names_file(id, args) { return Err(refused()); } let before = s.active().map(|d| d.doc.clone()); - let r = s.execute(id, args); + let r = execute_confined(s, id, args); let value = match r { Ok(v) => v, Err(e @ EngineError::Disabled(..)) => return Err(CallFailure { code: CALL_DISABLED, why: e.to_string() }), @@ -272,6 +298,13 @@ pub(crate) fn call(s: &mut Session, run: &Calls, id: &str, args: &Value) -> std: Ok(value) } +/// Runs command `id` with `args` in `s` with no file or folder readable or writable: every path the +/// command reaches through the checked file access ([`crate::file_access`]) is refused, whatever +/// param carries it. +fn execute_confined(s: &mut Session, id: &str, args: &Value) -> Result { + crate::file_access::confine(Some(&crate::file_access::AutomationRoots::none()), || s.execute(id, args)) +} + /// Runs `f`, a plug-in run that makes command calls, as one undo step named `label` in the active /// document: the commands it calls and the output it applies change the document inside one /// interaction, which is kept when `f` succeeds. When `f` fails, every document goes back to how it @@ -487,6 +520,51 @@ mod tests { assert!(!names_plugin(&json!({"effect": {"name": "plugin.style"}}))); } + /// A file or folder key counts at any depth, in objects and in arrays; a key [`NOT_FILES`] names + /// for the command only at the top level; ordinary nested params don't count. + #[test] + fn a_file_key_counts_at_any_depth() { + use serde_json::json; + let texture = json!({"effect": "distort.glass", "params": {"textureImage": {"path": "/tmp/t.png"}}}); + assert!(names_file("effect.apply", &texture)); + assert!(names_file("effect.setParams", &texture)); + assert!(names_file("effect.apply", &json!({"params": {"layers": [{"name": "a"}, {"folder": "/tmp"}]}}))); + assert!(names_file("select.all", &json!([[[{"url": "file:///etc/hosts"}]]]))); + assert!(!names_file("effect.apply", &json!({"effect": "distort.glass", "params": {"distortion": 5, "textureImage": "frosted"}}))); + assert!(!names_file("object.move", &json!({"dx": 5, "dy": 0, "ids": [1, 2], "opts": {"copy": false}}))); + // `path` is a shape's path in object.setLiveShape, but only as its own param. + assert!(!names_file("object.setLiveShape", &json!({"path": [[0, 0], [1, 1]]}))); + assert!(names_file("object.setLiveShape", &json!({"options": {"path": "/tmp/x"}}))); + assert!(names_file("object.move", &json!({"path": [[0, 0]]}))); + // Deep nesting is walked without recursion. + let mut deep = json!({"file": "/tmp/x"}); + for _ in 0..10_000 { + deep = Value::Array(vec![deep]); + } + assert!(names_file("select.all", &deep)); + std::mem::forget(deep); + } + + /// A call reads no file even when a param the checks above don't know carries its path: the + /// command runs with no file readable. Run as an ordinary request, the same texture loads. + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn a_confined_call_reads_no_file() { + use serde_json::json; + let file = std::env::temp_dir().join(format!("vc-plugin-call-texture-{}.png", std::process::id())); + let png = vectorcraft_trace::Raster::from_fn(8, 8, |x, y| if (x + y) % 2 == 0 { [0, 0, 0, 255] } else { [255, 255, 255, 255] }).encode_png(); + std::fs::write(&file, png).unwrap(); + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 50})).unwrap(); + let p = json!({"effect": "distort.glass", "params": {"textureImage": {"path": file.to_string_lossy()}}}); + let e = execute_confined(&mut s, "effect.apply", &p).map(|_| ()).unwrap_err().to_string(); + let read = s.execute("effect.apply", &p).map(|_| ()); + let _ = std::fs::remove_file(&file); + assert!(e.contains("read authority is absent"), "{e}"); + read.unwrap(); + } + #[test] fn keys_are_found_as_whole_keys() { assert!(names_key("{path: a .wasm file}", "path")); diff --git a/crates/engine/src/file_access.rs b/crates/engine/src/file_access.rs index cdea7ac5b..c6daff9e4 100644 --- a/crates/engine/src/file_access.rs +++ b/crates/engine/src/file_access.rs @@ -85,6 +85,12 @@ impl AutomationRoots { Ok(Some(Arc::new(Self { read: open(read, Access::Read)?, write: open(write, Access::Write)? }))) } + /// Roots that grant no access of either kind: inside [`confine`] with them, every path a + /// command reads or writes is refused. + pub(crate) fn none() -> Arc { + Arc::new(Self { read: Vec::new(), write: Vec::new() }) + } + /// May the file or folder at `path` be read (or written)? It must lie inside one of the roots /// of that kind. `Err` says why not. pub fn check(&self, path: &str, access: Access) -> Result<(), String> { diff --git a/crates/engine/tests/plugin_call.rs b/crates/engine/tests/plugin_call.rs index f30a8b812..71f2619a3 100644 --- a/crates/engine/tests/plugin_call.rs +++ b/crates/engine/tests/plugin_call.rs @@ -176,6 +176,51 @@ fn without_doc_write_a_call_that_edits_is_refused() { run(&mut s, "plugin.remove", json!({"id": "test.call.reader"})); } +/// Installs filter `id` that may read and change the document and call `allowlist`, whose `vc_run` +/// makes `calls` (command id and arguments) in order, dropping their codes, then returns nothing. +fn install_calls(s: &mut Session, id: &str, allowlist: &str, calls: &[(&str, &str)]) { + let mut imports = r#"(import "env" "vc_call" (func $vc_call (param i32 i32 i32 i32) (result i32)))"#.to_string(); + let mut first = String::new(); + for (i, (cmd, args)) in calls.iter().enumerate() { + let at = 0x3000 + i * 0x400; + imports += &format!(r#" (data (i32.const {at}) "{}") (data (i32.const {}) "{}")"#, tpl::string(cmd), at + 0x100, tpl::string(args)); + first += &format!("(drop (call $vc_call (i32.const {at}) (i32.const {}) (i32.const {}) (i32.const {})))", cmd.len(), at + 0x100, args.len()); + } + let manifest = format!( + r#"{{"id":"{id}","name":"Test {id}","kind":"filter","api":2,"permissions":["doc.read","doc.write","commands"],"commandAllowlist":{allowlist}}}"# + ); + let wasm = wat::parse_str(tpl::with_imports(&manifest, &imports, &first, EMPTY)).unwrap(); + let data = vectorcraft_format::base64_encode(&wasm); + let grant = json!(["doc.read", "doc.write", "commands"]); + run(s, "plugin.install", json!({"dataBase64": data, "grant": grant, "replace": true})); +} + +/// Arguments that carry a file or folder key at any depth, such as an effect's +/// `textureImage.path`, are refused before the command runs, so no file is read; arguments nested +/// without one run. +#[test] +fn a_call_whose_arguments_name_a_file_is_refused() { + let mut s = document(); + let before = state(&mut s); + let allowlist = r#"["select.all","effect.apply","effect.setParams"]"#; + let select = ("select.all", ""); + for (cmd, args) in [ + ("effect.apply", r#"{"effect":"distort.glass","params":{"textureImage":{"path":"missing-texture.png"}}}"#), + ("effect.setParams", r#"{"index":0,"params":{"textureImage":{"path":"missing-texture.png"}}}"#), + ("effect.apply", r#"{"effect":"distort.glass","params":{"layers":[{"name":"a"},{"file":"missing-texture.png"}]}}"#), + ] { + install_calls(&mut s, "test.call.files", allowlist, &[select, (cmd, args)]); + let e = run_plugin(&mut s, "test.call.files").unwrap_err(); + assert!(e.contains("may not call") && e.contains(cmd), "{args}: {e}"); + assert_eq!(state(&mut s), before, "{args}"); + } + // The same effect with ordinary nested params runs. + install_calls(&mut s, "test.call.files", allowlist, &[select, ("effect.apply", r#"{"effect":"distort.glass","params":{"distortion":5}}"#)]); + run_plugin(&mut s, "test.call.files").unwrap(); + assert_eq!(state(&mut s).1, before.1 + 1, "one undo step"); + run(&mut s, "plugin.remove", json!({"id": "test.call.files"})); +} + /// A command's result describes the document, so a plug-in without `doc.read` may call nothing. #[test] fn without_doc_read_every_call_is_refused() { diff --git a/docs/plugins.md b/docs/plugins.md index 51ec220eb..fb92da1d1 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -422,6 +422,10 @@ the module's start function, or in a run that isn't a filter's run on a document whole app, and for a command VectorCraft adds later until it has been checked for plug-in calls. Installing a manifest that lists such a command is refused, and the error names it; `plugin.inspect` lists them as `refusedCommands`. +- A call whose arguments hold a file or folder key (`path`, `paths`, `file`, `files`, `folder`, `folders`, `dir`, + `directory`, `destinationFolder`, `sourceFolder` or `url`) at any depth, such as an effect's `textureImage.path`, + returns `-6`; only a key that names part of the document in that command, as its own param, is let through. A + command a plug-in calls can't read or write any file or folder. - An id in `commandAllowlist` that no command has installs, so a plug-in can list a command a later version adds; `plugin.install` and `plugin.inspect` list such ids as `unknownCommands`, and calling one returns `-5` (`-6` without `doc.read`).