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.
This commit is contained in:
Dylan M. Taylor
2026-11-03 11:33:04 +00:00
parent 7870b5916a
commit ec653f8a91
4 changed files with 52 additions and 10 deletions
+6 -1
View File
@@ -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;
}
+2 -1
View File
@@ -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
+27 -3
View File
@@ -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"));
}
}
+17 -5
View File
@@ -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);
}
}
}