diff --git a/xtask/src/assets.rs b/xtask/src/assets.rs index 777dd652ef..cfec240bc6 100644 --- a/xtask/src/assets.rs +++ b/xtask/src/assets.rs @@ -21,15 +21,24 @@ pub fn glob(pat: &str, path: &str) -> bool { if let Some(dir) = pat.strip_suffix('/') { return path.starts_with(dir) && path[dir.len()..].starts_with('/'); } - fn rec(p: &[u8], s: &[u8]) -> bool { - match (p.first(), s.first()) { - (None, None) => true, - (Some(b'*'), _) => rec(&p[1..], s) || (!s.is_empty() && s[0] != b'/' && rec(p, &s[1..])), - (Some(a), Some(b)) if a == b => rec(&p[1..], &s[1..]), - _ => false, + // (pattern, path) positions already found not to match: each is tried once, so overlapping + // `*`s can't make the search exponential + fn rec(p: &[u8], s: &[u8], failed: &mut std::collections::HashSet<(usize, usize)>) -> bool { + if failed.contains(&(p.len(), s.len())) { + return false; } + let ok = match (p.first(), s.first()) { + (None, None) => true, + (Some(b'*'), _) => rec(&p[1..], s, failed) || (!s.is_empty() && s[0] != b'/' && rec(p, &s[1..], failed)), + (Some(a), Some(b)) if a == b => rec(&p[1..], &s[1..], failed), + _ => false, + }; + if !ok { + failed.insert((p.len(), s.len())); + } + ok } - rec(pat.as_bytes(), path.as_bytes()) + rec(pat.as_bytes(), path.as_bytes(), &mut std::collections::HashSet::new()) } fn backticked(cell: &str) -> impl Iterator { @@ -117,6 +126,13 @@ mod tests { assert!(glob("docs/images/*-tetons.jpg", "docs/images/ba-tetons.jpg")); assert!(!glob("docs/images/*.jpg", "docs/images/sub/x.jpg")); assert!(glob("crates/scenes/", "crates/scenes/a/b.png")); + // forty `*` against a long name that fails at its very end: exponential without the memo + let pat = format!("{}x", "*a".repeat(40)); + let name = "a".repeat(200); + let t = std::time::Instant::now(); + assert!(!glob(&pat, &name)); + assert!(t.elapsed().as_secs() < 5, "took {:?}", t.elapsed()); + assert!(glob("*a*b*", "xaybz") && !glob("*a*b*", "xa/b")); assert!(!glob("crates/scenes/", "crates/scenesx/b.png")); } diff --git a/xtask/src/ci_lock.rs b/xtask/src/ci_lock.rs index 9504beb20f..70c90b043b 100644 --- a/xtask/src/ci_lock.rs +++ b/xtask/src/ci_lock.rs @@ -74,7 +74,17 @@ pub fn try_acquire(dir: &Path, slots: usize, owner: &str) -> Result Ok(()) => { // a sidecar file, not the lock file: Windows locks are mandatory, so others couldn't read it let owner_file = owner_path(&path); - let _ = std::fs::File::create(&owner_file).and_then(|mut f| f.write_all(owner.as_bytes())); + // a new file: an entry left under the name (a link planted in a shared lock folder) + // is removed, never written through + let removed = std::fs::remove_file(&owner_file); + let written = OpenOptions::new().write(true).create_new(true).open(&owner_file).and_then(|mut f| f.write_all(owner.as_bytes())); + // when the file could be neither replaced nor written it may still name an earlier + // holder: say so rather than let `holders` report it as this run's slot's owner + if let (Err(e), Err(w)) = (&removed, &written) + && e.kind() != std::io::ErrorKind::NotFound + { + eprintln!("ci lock: could not record the owner in {} ({e}; {w}): waiting runs may show a stale owner", owner_file.display()); + } return Ok(Some(Held { _file: file, owner: owner_file })); } Err(TryLockError::WouldBlock) => continue, diff --git a/xtask/src/web.rs b/xtask/src/web.rs index f10ba489b5..c931917e9a 100644 --- a/xtask/src/web.rs +++ b/xtask/src/web.rs @@ -232,6 +232,21 @@ fn serve_dir(dir: &Path, port: u16) -> Result<(), String> { Ok(()) } +/// Whether a request path names a file inside the site folder: relative, made of plain names +/// only (no `..`, no root, no `C:` drive or other `:`, which `Path::join` on Windows would take as +/// the whole path). +fn safe_site_path(rel: &str) -> bool { + !rel.contains(['\\', ':']) && Path::new(rel).components().all(|c| matches!(c, std::path::Component::Normal(_))) +} + +/// A regular file's bytes (a symlink, folder or device under the name is refused, not read). +fn read_site_file(path: &Path) -> std::io::Result> { + if !std::fs::symlink_metadata(path)?.is_file() { + return Err(std::io::Error::other("not a file")); + } + std::fs::read(path) +} + fn handle(mut stream: TcpStream, dir: &Path) -> std::io::Result<()> { let mut reader = BufReader::new(stream.try_clone()?); let mut line = String::new(); @@ -253,11 +268,14 @@ fn handle(mut stream: TcpStream, dir: &Path) -> std::io::Result<()> { let rel = path.trim_start_matches('/'); let rel = if rel.is_empty() { "index.html" } else { rel }; let file = dir.join(rel); - let safe = !rel.split('/').any(|c| c == ".." || c.contains('\\')); + let safe = safe_site_path(rel); let encoding = if safe { negotiate(&accept_encoding, |suffix| dir.join(format!("{rel}{suffix}")).is_file()) } else { None }; - let read = match encoding { - Some((_, suffix)) => std::fs::read(dir.join(format!("{rel}{suffix}"))), - None => std::fs::read(&file), + // only a plain file of the site, read after the path check: a device or a path outside the + // folder is never opened + let read = match (safe, encoding) { + (false, _) => Err(std::io::Error::other("unsafe path")), + (true, Some((_, suffix))) => read_site_file(&dir.join(format!("{rel}{suffix}"))), + (true, None) => read_site_file(&file), }; let (status, body, ctype) = match (method, safe, read) { ("GET" | "HEAD", true, Ok(b)) => ("200 OK", b, mime(&file)), @@ -288,6 +306,14 @@ fn handle(mut stream: TcpStream, dir: &Path) -> std::io::Result<()> { mod tests { use super::*; + #[test] + fn served_paths_stay_inside_the_site() { + assert!(safe_site_path("index.html") && safe_site_path("pkg/lightcraft.js")); + for bad in ["../Cargo.toml", "a/../../x", "/etc/passwd", "C:/Windows/win.ini", "C:\\x", "a\\..\\b", "./x"] { + assert!(!safe_site_path(bad), "{bad}"); + } + } + #[test] fn bindgen_version_is_locked() { let v = locked_bindgen_version().unwrap();