From ec653f8a91e0f70efba6e54203f859775d9876d8 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 11:33:04 +0000 Subject: [PATCH] xtask serve, macOS cert import, audio probe test, Ease Presets: confine paths and temp files serve_dir stripped '..' and leading slashes but could still be steered outside the dist folder ('/..//etc/passwd' left '//etc/passwd', which join treats as absolute); it now keeps only normal path components. Each connection's thread blocked forever on a silent client and thread creation failure panicked; reads now time out and a failed spawn drops the connection. import-cert.sh wrote the signing bundle to a predictable path that another user could pre-create; mktemp makes it new and private. The audio probe test trusted a predictable, possibly pre-existing scratch dir for executables on PATH; it now creates a fresh private one. Ease Presets looked up each loaded preset name in the whole list (quadratic); a name set replaces it. --- extensions/scriptui-panels/Ease Presets.jsx | 7 ++++- packaging/macos/import-cert.sh | 3 ++- xtask/src/main.rs | 30 ++++++++++++++++++--- xtask/tests/it/audio_probe.rs | 22 +++++++++++---- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/extensions/scriptui-panels/Ease Presets.jsx b/extensions/scriptui-panels/Ease Presets.jsx index 9f4ea2185..8bffb335a 100644 --- a/extensions/scriptui-panels/Ease Presets.jsx +++ b/extensions/scriptui-panels/Ease Presets.jsx @@ -94,10 +94,15 @@ return out; } var list = doc && doc.presets instanceof Array ? doc.presets : []; + // Names taken so far, by lower case: looking each one up in the list made loading quadratic. + var taken = {}; + for (var b = 0; b < BUILT_IN.length; b++) taken["k" + BUILT_IN[b].name.toLowerCase()] = true; for (var i = 0; i < list.length; i++) { var name = cleanName(list[i] && list[i].name); var c = sanitized(list[i] && list[i].curve); - if (name && c && find(BUILT_IN, name) < 0 && find(out, name) < 0) out.push({ name: name, curve: c }); + if (!name || !c || taken["k" + name.toLowerCase()]) continue; + taken["k" + name.toLowerCase()] = true; + out.push({ name: name, curve: c }); } return out; } diff --git a/packaging/macos/import-cert.sh b/packaging/macos/import-cert.sh index 8e3462cde..6eda9a45f 100755 --- a/packaging/macos/import-cert.sh +++ b/packaging/macos/import-cert.sh @@ -19,7 +19,8 @@ fi TMP="${RUNNER_TEMP:-${TMPDIR:-/tmp}}" KEYCHAIN="$TMP/effectcraft-signing.keychain-db" -CERT="$TMP/effectcraft-signing.p12" +# Created new and private (mode 0600) by mktemp, never an existing file someone else can read. +CERT="$(mktemp "$TMP/effectcraft-signing.XXXXXX")" KC_PASS="${KEYCHAIN_PASSWORD:-$(openssl rand -hex 24)}" trap 'rm -f "$CERT"' EXIT diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 266f5acc3..d6e4b5727 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -362,7 +362,10 @@ fn serve_dir(dir: &std::path::Path, port: u16) -> Result<(), String> { println!("serving {} on http://127.0.0.1:{port}/", dir.display()); for stream in listener.incoming().flatten() { let dir = dir.to_path_buf(); - std::thread::spawn(move || { + // A client that connects and sends nothing gives its thread back after the timeout. + let _ = stream.set_read_timeout(Some(std::time::Duration::from_secs(10))); + // Out of threads: drop this connection rather than panic the server. + let _ = std::thread::Builder::new().spawn(move || { let mut stream = stream; let mut line = String::new(); let mut reader = BufReader::new(match stream.try_clone() { @@ -379,8 +382,7 @@ fn serve_dir(dir: &std::path::Path, port: u16) -> Result<(), String> { } } let path = line.split_whitespace().nth(1).unwrap_or("/").split(['?', '#']).next().unwrap_or("/").to_string(); - let rel = if path == "/" { "index.html".to_string() } else { path.trim_start_matches('/').replace("..", "") }; - let file = dir.join(&rel); + let file = served_file(&dir, &path); let (status, body) = match std::fs::read(&file) { Ok(b) => ("200 OK", b), Err(_) => ("404 Not Found", b"not found".to_vec()), @@ -408,6 +410,13 @@ fn serve_dir(dir: &std::path::Path, port: u16) -> Result<(), String> { Ok(()) } +/// The file under `dir` a request for `path` serves: only its normal components are kept (no +/// `..`, root, drive or empty ones), so the result stays inside `dir`. +fn served_file(dir: &std::path::Path, path: &str) -> std::path::PathBuf { + let parts: Vec<&str> = path.split(['/', '\\']).filter(|c| !c.is_empty() && *c != "." && *c != ".." && !c.contains(':')).collect(); + if parts.is_empty() { dir.join("index.html") } else { parts.iter().fold(dir.to_path_buf(), |p, c| p.join(c)) } +} + /// The guidance printed when the ALSA pkg-config package is missing. Split out so the tests can /// check the package names without needing a machine that is actually missing them. #[cfg(target_os = "linux")] @@ -596,3 +605,18 @@ mod tests { assert!(e.contains("pkg-config said: Package 'alsa' not found"), "detail lost:\n{e}"); } } + +#[cfg(test)] +mod serve_tests { + #[test] + fn served_files_stay_inside_the_served_folder() { + let dir = std::path::Path::new("/srv/dist"); + for p in ["/..//etc/passwd", "/....//....//etc/passwd", "//etc/passwd", "/..\\..\\x", "/C:/Windows/win.ini", "/a/../../b"] { + let f = super::served_file(dir, p); + assert!(f.starts_with(dir) && f.components().count() > dir.components().count(), "{p} -> {}", f.display()); + assert!(!f.components().any(|c| matches!(c, std::path::Component::ParentDir)), "{p} -> {}", f.display()); + } + assert_eq!(super::served_file(dir, "/"), dir.join("index.html")); + assert_eq!(super::served_file(dir, "/pkg/app.wasm"), dir.join("pkg").join("app.wasm")); + } +} diff --git a/xtask/tests/it/audio_probe.rs b/xtask/tests/it/audio_probe.rs index d984c7637..5a82e289c 100644 --- a/xtask/tests/it/audio_probe.rs +++ b/xtask/tests/it/audio_probe.rs @@ -18,11 +18,21 @@ struct FakeTools { impl FakeTools { fn new(tag: &str) -> Self { - let base = std::env::temp_dir().join(format!("effectcraft-xtask-audio-{tag}-{}", std::process::id())); + // A new directory this test creates itself: a predictable one that already exists (made + // by another user) is never trusted for the tools put on the child's PATH. + let base = (0u32..) + .map(|n| std::env::temp_dir().join(format!("effectcraft-xtask-audio-{tag}-{}-{n}", std::process::id()))) + .find(|b| std::fs::create_dir(b).is_ok()) + .expect("create a new scratch dir"); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&base, std::fs::Permissions::from_mode(0o700)).expect("make the scratch dir private"); + } let dir = base.join("bin"); let off_path = base.join("off-path"); - std::fs::create_dir_all(&dir).expect("create PATH dir"); - std::fs::create_dir_all(&off_path).expect("create off-PATH dir"); + std::fs::create_dir(&dir).expect("create PATH dir"); + std::fs::create_dir(&off_path).expect("create off-PATH dir"); Self { dir, off_path } } @@ -49,8 +59,10 @@ impl FakeTools { impl Drop for FakeTools { fn drop(&mut self) { - let _ = std::fs::remove_dir_all(&self.dir); - let _ = std::fs::remove_dir_all(&self.off_path); + // Both live in the scratch dir this test created. + if let Some(base) = self.dir.parent() { + let _ = std::fs::remove_dir_all(base); + } } }