diff --git a/apps/gridcraft-cli/src/check.rs b/apps/gridcraft-cli/src/check.rs index 74d2fca71..716f214b8 100644 --- a/apps/gridcraft-cli/src/check.rs +++ b/apps/gridcraft-cli/src/check.rs @@ -248,10 +248,11 @@ impl Discovery { /// (`~$…`) are skipped. fn collect_inputs(inputs: &[String]) -> Discovery { let mut out = Discovery::default(); + let mut seen = std::collections::HashSet::new(); for i in inputs { let p = PathBuf::from(i); if p.is_dir() { - if !walk(&p, 0, &mut out) { + if !walk(&p, 0, &mut out, &mut seen) { break; } } else if !out.add_file(p) { @@ -268,7 +269,20 @@ fn collect_inputs(inputs: &[String]) -> Discovery { } /// Returns false only when the global file budget stops traversal. -fn walk(dir: &Path, depth: usize, out: &mut Discovery) -> bool { +fn walk(dir: &Path, depth: usize, out: &mut Discovery, seen: &mut std::collections::HashSet) -> bool { + // A directory is visited once by identity (dev+inode on unix): several links to the same + // directory are aliases, not exponential branches of the scan. Other platforms keep the + // depth cap only. + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt as _; + if let Ok(m) = std::fs::metadata(dir) + && let Some(id) = m.dev().checked_mul(1 << 60).and_then(|d| d.checked_add(m.ino())) + && !seen.insert(id) + { + return true; + } + } if depth > MAX_DEPTH { out.fail(dir, format!("directory depth limit of {MAX_DEPTH} exceeded; this subtree was not checked")); return true; @@ -301,7 +315,7 @@ fn walk(dir: &Path, depth: usize, out: &mut Discovery) -> bool { } }; if metadata.is_dir() { - if !walk(&p, depth + 1, out) { + if !walk(&p, depth + 1, out, seen) { return false; } } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("xlsx") || e.eq_ignore_ascii_case("xlsm")) @@ -981,6 +995,12 @@ fn relationship_kinds(bytes: &[u8]) -> Result, String> let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).map_err(|e| format!("not a zip package: {e}"))?; let names = zip_names(&mut zip); let by_key: HashMap = names.iter().map(|n| (part_key(n), n)).collect(); + // Several relationships name the same part; each fresh check decompressed it again, so + // repeated references to one highly compressible part multiplied the work. The verdict is + // remembered per (part, named content) and the decompression the package can cause is + // budgeted as a whole (finding: repeated decompression). + let mut verdicts: HashMap<(String, &'static str), bool> = HashMap::new(); + let mut budget: u64 = 512 * 1024 * 1024; let mut kinds = BTreeMap::new(); for n in names.iter().filter(|n| part_key(n).ends_with(".rels")) { let Some(src_dir) = rels_source_dir(&part_key(n)) else { continue }; @@ -994,7 +1014,7 @@ fn relationship_kinds(bytes: &[u8]) -> Result, String> let external = attr(attrs, "TargetMode").is_some_and(|m| m.eq_ignore_ascii_case("External")); if !external && let Some(part) = by_key.get(&resolve_target(&src_dir, target).to_ascii_lowercase()) - && !holds_something(&mut zip, part, kind) + && !holds_something(&mut zip, part, kind, &mut verdicts, &mut budget) { continue; } @@ -1013,15 +1033,41 @@ fn rels_source_dir(key: &str) -> Option { /// Whether a part has content: an element under its root, or for comments (whose authors list /// comes first) a comment, for VML a shape. Read only as far as needed; an unreadable part counts. -fn holds_something(zip: &mut Zip<'_>, name: &str, kind: &str) -> bool { - use quick_xml::events::Event; - let Ok(f) = zip.by_name(name) else { return true }; - let mut reader = quick_xml::Reader::from_reader(std::io::BufReader::new(f.take(MAX_XML_PART))); - let wanted: Option<&[u8]> = match kind.to_ascii_lowercase().as_str() { +/// `verdicts` remembers the answer per (part, named content); `budget` is the decompression this +/// package's remaining checks may cause, and once it is spent the part counts as having content. +fn holds_something(zip: &mut Zip<'_>, name: &str, kind: &str, verdicts: &mut HashMap<(String, &'static str), bool>, budget: &mut u64) -> bool { + let wanted: Option<&'static [u8]> = match kind.to_ascii_lowercase().as_str() { "comments" => Some(b"comment"), "vmldrawing" => Some(b"shape"), _ => None, }; + let marker = match wanted { + Some(b"comment") => "comment", + Some(_) => "shape", + None => "any", + }; + if let Some(v) = verdicts.get(&(name.to_ascii_lowercase(), marker)) { + return *v; + } + let v = holds_something_uncached(zip, name, marker, budget); + verdicts.insert((name.to_ascii_lowercase(), marker), v); + v +} + +fn holds_something_uncached(zip: &mut Zip<'_>, name: &str, marker: &'static str, budget: &mut u64) -> bool { + use quick_xml::events::Event; + let Ok(f) = zip.by_name(name) else { return true }; + if *budget == 0 { + return true; + } + let take = f.size().min(MAX_XML_PART).min(*budget); + *budget -= take; + let mut reader = quick_xml::Reader::from_reader(std::io::BufReader::new(f.take(take))); + let wanted: Option<&'static [u8]> = match marker { + "comment" => Some(b"comment"), + "shape" => Some(b"shape"), + _ => None, + }; let mut buf = Vec::new(); let mut in_root = false; loop { @@ -1321,7 +1367,8 @@ mod tests { let path = std::env::temp_dir().join(format!("gridcraft-check-not-a-directory-{}", std::process::id())); std::fs::write(&path, "not a directory").unwrap(); let mut discovery = Discovery::default(); - assert!(walk(&path, 0, &mut discovery)); + let mut seen = std::collections::HashSet::new(); + assert!(walk(&path, 0, &mut discovery, &mut seen)); assert!(discovery.files.is_empty()); assert_eq!(discovery.failures.len(), 1); assert_eq!(discovery.failures[0].path, path.to_string_lossy()); diff --git a/apps/gridcraft-web/src/web.rs b/apps/gridcraft-web/src/web.rs index 8e2e60837..bae684c38 100644 --- a/apps/gridcraft-web/src/web.rs +++ b/apps/gridcraft-web/src/web.rs @@ -214,7 +214,9 @@ fn services(inbox: Inbox, ctx: egui::Context) -> Services { if let Some(w) = web_sys::window() && let gridcraft_engine::links::LinkKind::External(url) = gridcraft_engine::links::classify(url) { - let _ = w.open_with_url_and_target(&url, "_blank"); + // With `noopener` the new browsing context can't hold a reference back to the + // editor window and navigate it while the user reads the linked page. + let _ = w.open_with_url_and_target(&url, "_blank,noopener"); } })), inbox: Some(inbox), diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs index c8e3168db..4bdb3a30d 100644 --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -22,6 +22,11 @@ const FULL_SIZE_CELLS: u64 = 16_384; /// range, so that formulas over them stay fast. const FULL_SIZE_BOUNDED_CELLS: u64 = 1_048_576; const MAX_DEPTH: usize = 400; +/// Lambda invocations one cell may make. A semantically bounded formula in Excel can invoke +/// its lambda at most once per cell of an array (capped here by `MAX_CELLS`), so this covers +/// every legitimate call; hostile work that grows past one dimension (a LAMBDA that recurses +/// or nest `MAP` into `MAP`) then stops at the same bound instead of squaring. +const LAMBDA_FUEL: u64 = MAX_CELLS; #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub struct Area { @@ -117,6 +122,11 @@ pub struct Evaluator<'h> { calls: usize, /// `stack_position()` at the outermost `eval`. stack_top: usize, + /// Lambda invocations this cell still has left. Every call spends one charge, so a + /// compact hostile formula (a LAMBDA that calls itself, nested `MAP`s over big arrays) + /// runs out of work after the same number of calls that a legitimately wide one could + /// make, instead of an amount of work that squares with array nesting (#2382). + fuel: u64, } struct FnCtx<'a, 'h> { @@ -164,8 +174,12 @@ impl Ctx for FnCtx<'_, '_> { depth: self.ev.depth, calls: self.ev.calls, stack_top: self.ev.stack_top, + fuel: self.ev.fuel, }; - Some(matches!(ev.value(&shifted).scalar().to_bool(), Ok(true))) + let v = matches!(ev.value(&shifted).scalar().to_bool(), Ok(true)); + // The child's spend counts against this cell's budget. + self.ev.fuel = ev.fuel; + Some(v) } fn has_lambda(&self) -> bool { self.lambda.is_some() @@ -184,7 +198,7 @@ impl Ctx for FnCtx<'_, '_> { impl<'h> Evaluator<'h> { pub fn new(host: &'h mut dyn Host, sheet: usize, at: CellRef) -> Self { - Evaluator { host, sheet, at, env: Vec::new(), depth: 0, calls: 0, stack_top: 0 } + Evaluator { host, sheet, at, env: Vec::new(), depth: 0, calls: 0, stack_top: 0, fuel: LAMBDA_FUEL } } /// Evaluates to a value (references are dereferenced; multi-cell → array). @@ -1032,6 +1046,13 @@ impl<'h> Evaluator<'h> { if self.calls >= MAX_LAMBDA_CALLS { return err(CellError::Num); } + // #2382: one charge per invocation, so a cell's total work stays bounded however the + // lambdas nest. Foregrounding the charge here means every path into a body (direct + // `(2,3)` application, `MAP`/`REDUCE`/`BYROW` helpers, D-function criteria) meters. + self.fuel = match self.fuel.checked_sub(1) { + Some(f) => f, + None => return err(CellError::Calc), + }; self.calls += 1; let saved = std::mem::replace(&mut self.env, l.env.clone()); self.env.extend(bound); @@ -2035,3 +2056,54 @@ pub fn is_known_function(n: &str) -> bool { #[allow(dead_code)] fn unused(_: RefKind) {} + +#[cfg(test)] +mod fuel_tests { + use super::*; + use gridcraft_model::Workbook; + + struct Empty { + wb: Workbook, + } + impl Host for Empty { + fn workbook(&self) -> &Workbook { + &self.wb + } + fn cell_value(&mut self, _sheet: usize, _c: CellRef) -> Value { + Value::Empty + } + fn now_serial(&self) -> f64 { + 0.0 + } + fn random(&mut self) -> f64 { + 0.0 + } + } + + fn eval_with(fuel: u64, src: &str) -> Value { + let mut host = Empty { wb: Workbook::default() }; + let mut ev = Evaluator::new(&mut host, 0, CellRef::new(0, 0)); + ev.fuel = fuel; + ev.value(&gridcraft_formula::parser::parse(src).unwrap_or_else(|e| panic!("{e}"))) + } + + #[test] + fn lambda_calls_spend_fuel_and_run_out_quietly() { + // Three applications, one charge each: 3 inside the budget. (A nested MAP wouldn't do + // here: an inner MAP result is an array, and MAP of array elements is #CALC! on shape, + // which happens well before the budget does.) + let r = eval_with(3, "=LAMBDA(x,LAMBDA(y,LAMBDA(z,z+1)(3))(2))(1)"); + assert_eq!(r, Value::number(4.0), "{r:?}"); + // One charge short: the nest bottoms out in #CALC! instead of running unbounded. + let r = eval_with(2, "=LAMBDA(x,LAMBDA(y,LAMBDA(z,z+1)(3))(2))(1)"); + assert!(matches!(r, Value::Error(CellError::Calc)), "{r:?}"); + // One charge short of a single application: #CALC! with a zero budget. + let r = eval_with(0, "=LAMBDA(x,x+1)(2)"); + assert!(matches!(r, Value::Error(CellError::Calc)), "{r:?}"); + let r = eval_with(1, "=LAMBDA(x,x+1)(2)"); + assert_eq!(r, Value::number(3.0)); + // A flat helper map keeps working (3 of 3 invokes inside the budget). + let r = eval_with(3, "=MAP({1,2,3},LAMBDA(x,x*2))"); + assert!(matches!(r, Value::Array(_)) && !matches!(&r, Value::Array(a) if a.data.iter().any(|v| matches!(v, Value::Error(_)))), "{r:?}"); + } +} diff --git a/crates/calc/src/tests.rs b/crates/calc/src/tests.rs index f6ddd5db3..1720bcb57 100644 --- a/crates/calc/src/tests.rs +++ b/crates/calc/src/tests.rs @@ -570,6 +570,24 @@ fn lambda_helpers_check_shapes_and_parameters() { assert_eq!(t.num("B7"), 2.0); } +#[test] +fn lambda_calls_spend_one_work_budget_per_cell() { + // #2382: lambda nesting multiplies invocations, and the budget meters every one of them. + // Exhaustion itself is tested on a mock host in `eval::fuel_tests` (burning the real 16M + // budget in a full recalc needs tens of millions of calls, far too slow for a unit test); + // here the same nested shape runs inside it: 6 × 9 ≈ 59k scalar REDUCE invocations. + let mut t = T::new(); + t.set( + "A1", + "=LET(s,SEQUENCE(9),REDUCE(0,s,LAMBDA(a,v,REDUCE(0,s,LAMBDA(a,v,REDUCE(0,s,LAMBDA(a,v,REDUCE(0,s,LAMBDA(a,v,REDUCE(0,s,LAMBDA(a,v,a+v)))))))))))", + ); + // The bodies are the same inner reduce, so every level sums to 45 while the invocation + // count multiplies by 9 per level (9, 81, 729, 6561, 59049 calls). + assert_eq!(t.num("A1"), 45.0); + t.set("A2", "=REDUCE(0,SEQUENCE(4000),LAMBDA(a,v,a+v))"); + assert_eq!(t.num("A2"), 4000.0 * 4001.0 / 2.0); +} + #[test] fn accounting_negative_text_coerces() { let mut t = T::new(); diff --git a/crates/chart/src/raster.rs b/crates/chart/src/raster.rs index 67ed53ec6..0de2450b5 100644 --- a/crates/chart/src/raster.rs +++ b/crates/chart/src/raster.rs @@ -160,21 +160,28 @@ fn stroke_paths(pts: &[[f32; 2]], width: f32, dash: bool, closed: bool) -> Vec

0.0) { continue; } let mut t = 0.0; - let mut n = 0; - while t < len && n < 10_000 { + while t < len && budget > 0 { let e = (t + on).min(len); out.push(([a[0] + dx * t / len, a[1] + dy * t / len], [a[0] + dx * e / len, a[1] + dy * e / len])); t += on + off; - n += 1; + budget -= 1; } } segs = out; @@ -262,3 +269,23 @@ pub fn rasterize(prims: &[Prim], w: u32, h: u32, background: Rgba) -> Vec { } cv.buf } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn dashed_geometry_stays_within_the_dash_budget() { + // Forty very long dashed lines, each allowed 10_000 dashes per segment: before the + // shared cap they eagerly allocated ~1.6 MB of segment pairs × points and a like + // number of quads before any of it reached a 4-pixel canvas (finding: dash expansion). + let prims: Vec = + (0..40).map(|i| Prim::Line { pts: vec![[0.0, i as f32], [900.0, i as f32]], color: [0, 0, 0, 255], width: 1.0, dash: true }).collect(); + let img = rasterize(&prims, 2, 2, [255, 255, 255, 255]); + assert_eq!(img.len(), 2 * 2 * 4); + // And the geometry itself is bounded even for one huge dashed polyline. + assert!(stroke_paths(&[[0.0, 0.0], [1e6, 0.0]], 1.0, true, false).len() <= 65_536); + let prim = [Prim::Line { pts: vec![[0.0, 0.0], [10.0, 0.0]], color: [0, 0, 0, 255], width: 1.0, dash: true }]; + assert_eq!(rasterize(&prim, 1, 1, [0, 0, 0, 255]).len(), 4); + } +} diff --git a/crates/core/src/addr.rs b/crates/core/src/addr.rs index 45812b0b6..03c165a38 100644 --- a/crates/core/src/addr.rs +++ b/crates/core/src/addr.rs @@ -350,6 +350,21 @@ mod tests { assert_eq!(split_sheet("A1"), (None, "A1")); } + /// #2312: device/backslash garbage in front of `!` is cosmetic at worst — `split_sheet` only + /// splits at `!`, and every consumer matches the sheet part against workbook sheet names (it + /// never reaches a filesystem path or a shell); the body half always stays a pure cell/range + /// reference. + #[test] + fn split_sheet_leaves_device_names_inert_in_the_body() { + for s in ["C:\\a\\file!A1", "CON!A1", "NUL!A1", "a[b]!A1"] { + let (sheet, rest) = split_sheet(s); + assert_eq!(CellRef::parse(rest), Some(CellRef::new(0, 0)), "{s:?}: body must stay a bare reference"); + let _ = sheet; + } + // A plain single-letter sheet is still split, not mistaken for a body. + assert_eq!(split_sheet("C!A1").0, Some("C".to_string())); + } + #[test] fn letters_roundtrip() { for (c, s) in [(0, "A"), (25, "Z"), (26, "AA"), (51, "AZ"), (52, "BA"), (701, "ZZ"), (702, "AAA"), (16383, "XFD")] { diff --git a/crates/core/src/parse.rs b/crates/core/src/parse.rs index 7316d4222..a9901db39 100644 --- a/crates/core/src/parse.rs +++ b/crates/core/src/parse.rs @@ -371,10 +371,16 @@ fn parse_datetime(t: &str, sys: DateSystem) -> Option<(f64, Option<&'static str> if let Some((d, f)) = parse_date(t, sys) { return Some((d, Some(f))); } - // date + time: split at the last space(s) before a time-looking tail. + // date + time: split at the last space(s) before a time-looking tail. Only splits where the + // tail could be a time are tried: a long tail can't parse, so this stays linear in the text + // instead of reparsing every prefix for every space (one long text field quadratic). for (i, _) in t.match_indices(' ') { - let (a, b) = (t.get(..i)?, t.get(i + 1..)?); - if let (Some((d, _)), Some((tf, _, _))) = (parse_date(a, sys), parse_time(b)) { + let Some(b) = t.get(i + 1..) else { continue }; + let bytes = b.as_bytes(); + if b.len() > 36 || !bytes.first().is_some_and(u8::is_ascii_digit) || !b.contains(':') { + continue; + } + if let (Some((d, _)), Some((tf, _, _))) = (t.get(..i).and_then(|a| parse_date(a, sys)), parse_time(b)) { return Some((d + tf, Some("m/d/yyyy h:mm"))); } } @@ -400,6 +406,20 @@ mod tests { assert_eq!(parse_number_text("1 1/2"), Some(1.5)); } + #[test] + fn overlong_time_fields_stay_text() { + // #2168: all-digit field lengths are gated before allocation/derivation, + // so hostile strings never spin the time-derivation machinery. + let big = "9".repeat(500); + for s in [format!("{big}:00"), format!("1:00:{big}"), format!("{big}:{big}:{big}"), format!("1:0{big}"), format!("00:00:{big}.1")] { + assert_eq!(p(&s).value, Value::text(s.as_str()), "{s}"); + assert_eq!(parse_number_text(&s), None, "{s}"); + } + // Within the gate, times still work. + assert_eq!(p("01:02:03").value, Value::number(3.0 / 86_400.0 + 1.0 / 24.0 + 2.0 / 1440.0)); + assert_eq!(parse_number_text("13:14:15"), Some(0.5515625)); + } + #[test] fn non_digit_time_and_fraction_parts_stay_text() { for s in ["1:nan", "nan:30", "1:30:NaN", "1 nan/2", "1 1/nan", "inf:00", "1e1:00", "+5:00"] { diff --git a/crates/engine/src/clip_html.rs b/crates/engine/src/clip_html.rs index d208155df..e1d217c9c 100644 --- a/crates/engine/src/clip_html.rs +++ b/crates/engine/src/clip_html.rs @@ -267,6 +267,12 @@ fn attr<'a>(attrs: &'a str, name: &str) -> Option<&'a str> { } } +/// Entities are at most `&` + 8 chars + `;` (a decimal code point at most 7 digits deep), so +/// the semicolon is only looked for in the first 12 bytes — and `unwrap_or(rest)` never falls +/// back to scanning the remainder of the text either (that turn re-scans per `&`, quadratic +/// when a paste holds many bare `&`). +const ENTITY_WINDOW: usize = 12; + /// HTML text to plain text: entities decoded, whitespace runs (and `
`) collapsed to one /// space, trimmed. The text only serves to match a cell with the tab-separated text. fn decode(raw: &str) -> String { @@ -275,7 +281,9 @@ fn decode(raw: &str) -> String { while let Some(amp) = rest.find('&') { out.push_str(rest.get(..amp).unwrap_or_default()); rest = rest.get(amp..).unwrap_or_default(); - let semi = rest.get(..12).unwrap_or(rest).find(';'); + // `.min(len)` (not an `unwrap_or(rest)`) — a `get(..12)` that fails on a multibyte + // boundary used to fall back to the complete suffix, re-scanning per '&'. + let semi = rest.get(..ENTITY_WINDOW.min(rest.len())).and_then(|w| w.find(';')); let entity = semi.and_then(|e| rest.get(1..e)); let ch = entity.and_then(|e| match e { "amp" => Some('&'), @@ -382,6 +390,19 @@ mod tests { assert_eq!(table_cells(quoted_gt, 1, 1), vec![vec![HtmlCell { text: "1500".into(), num: Some(1500.0) }]]); } + #[test] + fn entities_recognized_only_within_the_twelve_byte_window() { + // The window never scans the rest of the text for a semicolon: a bare '&' followed by + // long plain text stays a bare '&', and one '&' after another is linear. + assert_eq!(decode("a&lifier; b"), "a&lifier; b"); + assert_eq!(decode("&ampampampAMP;"), "&ampampampAMP;"); + // Inside the window the usual forms still decode, and a trailing bare '&' too. + assert_eq!(decode("&"), "&"); + assert_eq!(decode("♪"), "\u{266A}"); + assert_eq!(decode("a&b c"), "a&b\u{a0}c"); + assert_eq!(decode("amp&"), "amp&"); + } + #[test] fn hostile_markup_does_not_panic() { for html in [ diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 009d77f2a..7396d5356 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -1034,6 +1034,11 @@ fn denied_script_step(id: &str, params: &Json) -> Option { if id.starts_with("file.") || id.starts_with("link.") { return Some(format!("scripts can't run `{id}`: it reaches outside the workbook")); } + // Scripts are scoped to their workbook: a step that switches the active window would + // point every later step at another open document, so those are refused too. + if id.starts_with("window.") { + return Some(format!("scripts can't run `{id}`: it reaches outside the workbook")); + } if params.get("path").is_some() { return Some(format!("scripts can't pass a file `path` to `{id}`")); } diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index cd28cfbfa..691b531c7 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -233,10 +233,23 @@ fn with(p: &Json, key: &str, v: Json) -> Json { pub(crate) fn apply_style(s: &mut Session, p: &Json, f: impl Fn(&mut Style)) -> Result { let sheet = target_sheet(s, p)?; let ranges = target_ranges(s, p)?; + let guarded = s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.protection.is_some()); if s.doc()?.wb.sheet(sheet).is_some_and(|sh| sh.protection.as_ref().is_some_and(|pr| !pr.format_cells)) { return Err(EngineError::Other("The cell or chart you're trying to change is on a protected sheet.".into())); } edit(s, |cx| { + // Formatting on a protected sheet keeps the cell's permission flags: a format call + // that flips `locked` from the format permission would authorize content edits that + // the password should still gate, so those flags may not change (Excel keeps that + // checkbox disabled while the sheet is protected). + const FLAG_GUARD: &str = "You can't change cell protection on a protected sheet. Unprotect it first."; + macro_rules! guard_flags { + ($old:expr, $new:expr) => { + if guarded && cx.wb.styles.get($old).protection != cx.wb.styles.get($new).protection { + return Err(EngineError::Other(FLAG_GUARD.into())); + } + }; + } let mut cache: std::collections::HashMap = std::collections::HashMap::new(); for r in &ranges { let full_rows = r.is_full_rows(); @@ -251,6 +264,7 @@ pub(crate) fn apply_style(s: &mut Session, p: &Json, f: impl Fn(&mut Style)) -> info.and_then(|i| i.style).unwrap_or_default() }; let new = *cache.entry(cur).or_insert_with(|| cx.wb.styles.derive(cur, &f)); + guard_flags!(cur, new); let sh = cx.sheet_mut(sheet)?; let map = if full_rows && !full_cols { &mut sh.rows } else { &mut sh.cols }; map.entry(line).or_default().style = Some(new); @@ -260,6 +274,7 @@ pub(crate) fn apply_style(s: &mut Session, p: &Json, f: impl Fn(&mut Style)) -> cx.wb.sheet(sheet).map(|sh| sh.cells.iter_range(*r).map(|(c, x)| (c, x.style)).collect()).unwrap_or_default(); for (c, st) in &cells { let new = *cache.entry(*st).or_insert_with(|| cx.wb.styles.derive(*st, &f)); + guard_flags!(*st, new); cx.sheet_mut(sheet)?.set_style(*c, new); } // Formatting touched these cells: re-fit their rows (Excel auto-fits). @@ -276,6 +291,7 @@ pub(crate) fn apply_style(s: &mut Session, p: &Json, f: impl Fn(&mut Style)) -> let st = cx.wb.sheet(sheet).map(|sh| sh.style_id(c)).unwrap_or_default(); let new = *cache.entry(st).or_insert_with(|| cx.wb.styles.derive(st, &f)); if new != st { + guard_flags!(st, new); cx.sheet_mut(sheet)?.set_style(c, new); } } diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 8a0bb1052..8b2899f27 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -552,9 +552,15 @@ pub fn auto_row_height(wb: &mut Workbook, si: usize, row: u32) { return; } let mut need: f32 = default; + // The cell loop must not rescan every merge per cell: the merges touching this row are + // picked out once, and per cell the candidates are re-checked (normally none or a couple; + // a sheet with many merges and many saved columns would otherwise scan all of them per + // cell, rows × merges × columns). + let row_merges: Vec = + sh.merges.iter().copied().filter(|m| !m.is_single() && m.start.row <= row && row <= m.end.row).collect(); for (c, cell) in sh.cells.row(row, 0, gridcraft_core::MAX_COLS - 1) { // Merged cells never auto-fit their rows, like Excel (column autofit skips them too). - if sh.merge_at(CellRef::new(row, c)).is_some_and(|m| !m.is_single()) { + if row_merges.iter().any(|m| m.contains(CellRef::new(row, c))) { continue; } let st = wb.styles.get(cell.style); diff --git a/crates/engine/src/cmd/print.rs b/crates/engine/src/cmd/print.rs index 4c80f2fa9..1c91518e2 100644 --- a/crates/engine/src/cmd/print.rs +++ b/crates/engine/src/cmd/print.rs @@ -784,22 +784,27 @@ fn wrap_lines(text: &str, font: Font, size: f32, width: f32) -> Vec { out.push(std::mem::take(&mut line)); line = word.to_string(); } - // Break a single over-long word by characters. + // Break a single over-long word by characters. The cut that fits is found by binary + // search (each try measures one candidate prefix), so an over-long word costs + // O(n log n) measurements instead of re-measuring the whole cut per character for + // every emitted line (`a` × 100k pasted text would be cubic worst case). while gridcraft_pdf::text_width(font, size, &line) > width && line.chars().count() > 1 { - let mut cut = String::new(); - let mut rest = String::new(); - for ch in line.chars() { - if rest.is_empty() && gridcraft_pdf::text_width(font, size, &format!("{cut}{ch}")) <= width || cut.is_empty() { - cut.push(ch); + let chars: Vec = line.chars().collect(); + let (mut lo, mut hi) = (1usize, chars.len()); + while lo < hi { + let mid = lo + (hi - lo).div_ceil(2); + let cut: String = chars[..mid].iter().collect(); + if gridcraft_pdf::text_width(font, size, &cut) <= width { + lo = mid; } else { - rest.push(ch); + hi = mid - 1; } } - if rest.is_empty() { + if lo == chars.len() { break; } - out.push(cut); - line = rest; + out.push(chars[..lo].iter().collect()); + line = chars[lo..].iter().collect(); } if out.len() > 2000 { return out; @@ -1306,7 +1311,16 @@ fn decode_image(data: &[u8], max_w: u32, max_h: u32) -> Option<(Vec, u32, u3 } /// Draws charts, pictures, shapes and sparklines overlapping the body block of a page. -fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet, b: &Block) { +/// `pics` memoizes decoded pictures for the whole render (one page at a time draws each +/// picture again, but the decode happens once). +fn draw_objects( + pg: &mut Page, + wb: &Workbook, + si: usize, + sh: &Sheet, + b: &Block, + pics: &mut std::collections::HashMap<(usize, usize), Option<(Vec, u32, u32)>>, +) { let block = b.range(); let (sx0, sy0) = (sh.col_left(block.start.col), sh.row_top(block.start.row)); let (sx1, sy1) = (sh.col_left(block.end.col.saturating_add(1)), sh.row_top(block.end.row.saturating_add(1))); @@ -1316,6 +1330,42 @@ fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet, b: &Block) let place = |r: (f64, f64, f64, f64)| (bx + ((r.0 - sx0) as f32) * b.k, by + ((r.1 - sy0) as f32) * b.k, r.2 as f32 * b.k, r.3 as f32 * b.k); pg.save(); pg.clip_rect(bx, by, bw, bh); + draw_sparklines(pg, wb, si, sh, b, block); + for (imi, im) in sh.images.iter().enumerate() { + let r = anchor_rect(sh, &im.anchor); + if !visible(r) { + continue; + } + let (x, y, w, h) = place(r); + // About 150 dpi is plenty for print. + let (mw, mh) = (((w / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32, ((h / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32); + let decoded = pics.entry((si, imi)).or_insert_with(|| decode_image(&im.data, mw, mh)); + if let Some((px, pw, ph)) = decoded.as_ref() { + pg.image_rgba(x, y, w, h, px, *pw, *ph); + } + } + for shp in &sh.shapes { + let r = anchor_rect(sh, &shp.anchor); + if !visible(r) { + continue; + } + let (x, y, w, h) = place(r); + draw_shape(pg, wb, shp, b, (x, y, w, h)); + } + for ch in &sh.charts { + let r = anchor_rect(sh, &ch.anchor); + if !visible(r) { + continue; + } + let (x, y, _, _) = place(r); + let data = gridcraft_chart::resolve(wb, si, ch); + let prims = gridcraft_chart::render(ch, &data, r.2 as f32, r.3 as f32, &PdfMeasure); + draw_prims(pg, &prims, Xf { ox: x, oy: y, k: b.k }); + } + pg.restore(); +} + +fn draw_sparklines(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet, b: &Block, block: RangeRef) { for sp in &sh.sparklines { if !block.contains(sp.cell) { continue; @@ -1330,90 +1380,64 @@ fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet, b: &Block) let prims = gridcraft_chart::render_sparkline(sp.kind, &vals, [color[0], color[1], color[2], 255], sp.markers, wpx, hpx); draw_prims(pg, &prims, Xf { ox: x, oy: y, k: b.k }); } - for im in &sh.images { - let r = anchor_rect(sh, &im.anchor); - if !visible(r) { - continue; +} + +fn draw_shape(pg: &mut Page, wb: &Workbook, shp: &gridcraft_model::Shape, b: &Block, (x, y, w, h): (f32, f32, f32, f32)) { + let fill = rgb_of(shp.fill, wb); + let line = rgb_of(shp.line, wb).filter(|_| !shp.format.no_line).map(|c| (c, (0.75 * b.k).max(0.25))); + match shp.kind { + ShapeKind::Ellipse => { + let pts: Vec<[f32; 2]> = (0..72) + .map(|i| { + let a = i as f32 / 72.0 * std::f32::consts::TAU; + [x + w / 2.0 + w / 2.0 * a.cos(), y + h / 2.0 + h / 2.0 * a.sin()] + }) + .collect(); + pg.polygon(&pts, fill, line); } - let (x, y, w, h) = place(r); - // About 150 dpi is plenty for print. - let (mw, mh) = (((w / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32, ((h / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32); - if let Some((px, pw, ph)) = decode_image(&im.data, mw, mh) { - pg.image_rgba(x, y, w, h, &px, pw, ph); + ShapeKind::Triangle => pg.polygon(&[[x + w / 2.0, y], [x + w, y + h], [x, y + h]], fill, line), + ShapeKind::Line | ShapeKind::Arrow => { + let c = line.map(|l| l.0).or(fill).unwrap_or([0, 0, 0]); + pg.stroke_line(x, y, x + w, y + h, (1.0 * b.k).max(0.3), c); + if shp.kind == ShapeKind::Arrow { + let ang = h.atan2(w); + let al = 8.0 * b.k; + let p = |da: f32| [x + w - al * (ang + da).cos(), y + h - al * (ang + da).sin()]; + pg.polygon(&[[x + w, y + h], p(0.4), p(-0.4)], Some(c), None); + } + } + _ => { + pg.polygon(&[[x, y], [x + w, y], [x + w, y + h], [x, y + h]], fill, line); } } - for shp in &sh.shapes { - let r = anchor_rect(sh, &shp.anchor); - if !visible(r) { - continue; - } - let (x, y, w, h) = place(r); - let fill = rgb_of(shp.fill, wb); - let line = rgb_of(shp.line, wb).filter(|_| !shp.format.no_line).map(|c| (c, (0.75 * b.k).max(0.25))); - match shp.kind { - ShapeKind::Ellipse => { - let pts: Vec<[f32; 2]> = (0..72) - .map(|i| { - let a = i as f32 / 72.0 * std::f32::consts::TAU; - [x + w / 2.0 + w / 2.0 * a.cos(), y + h / 2.0 + h / 2.0 * a.sin()] - }) - .collect(); - pg.polygon(&pts, fill, line); - } - ShapeKind::Triangle => pg.polygon(&[[x + w / 2.0, y], [x + w, y + h], [x, y + h]], fill, line), - ShapeKind::Line | ShapeKind::Arrow => { - let c = line.map(|l| l.0).or(fill).unwrap_or([0, 0, 0]); - pg.stroke_line(x, y, x + w, y + h, (1.0 * b.k).max(0.3), c); - if shp.kind == ShapeKind::Arrow { - let ang = h.atan2(w); - let al = 8.0 * b.k; - let p = |da: f32| [x + w - al * (ang + da).cos(), y + h - al * (ang + da).sin()]; - pg.polygon(&[[x + w, y + h], p(0.4), p(-0.4)], Some(c), None); - } - } - _ => { - pg.polygon(&[[x, y], [x + w, y], [x + w, y + h], [x, y + h]], fill, line); - } - } - if !shp.text.is_empty() { - use gridcraft_model::{ShapeTextAlign, ShapeTextAnchor}; - let fm = &shp.format; - let text_box = shp.kind == ShapeKind::TextBox; - let size = fm.font_size.unwrap_or(11.0).clamp(1.0, 4000.0) * b.scale; - let font = Font::styled(Font::family_for(fm.font.as_deref().unwrap_or("")), fm.bold, fm.italic); - let color = fm.text_color.and_then(|c| rgb_of(c, wb)).unwrap_or([0, 0, 0]); - let align = fm.align.unwrap_or(if text_box { ShapeTextAlign::Left } else { ShapeTextAlign::Center }); - let anchor = fm.anchor.unwrap_or(if text_box { ShapeTextAnchor::Top } else { ShapeTextAnchor::Middle }); - let lines = wrap_lines(&shp.text, font, size, (w - 4.0 * b.k).max(size)); - let block_h = lines.len().min(500) as f32 * size * 1.2; - let mut yy = match anchor { - ShapeTextAnchor::Top => y + 2.0 * b.k, - ShapeTextAnchor::Middle => y + (h - block_h) / 2.0, - ShapeTextAnchor::Bottom => y + h - 2.0 * b.k - block_h, - } + size * 0.8; - for l in lines.iter().take(500) { - let lw = gridcraft_pdf::text_width(font, size, l); - let lx = match align { - ShapeTextAlign::Left => x + 2.0 * b.k, - ShapeTextAlign::Center => x + (w - lw) / 2.0, - ShapeTextAlign::Right => x + w - 2.0 * b.k - lw, - }; - pg.text(lx, yy, size, font, color, l); - yy += size * 1.2; - } - } + if shp.text.is_empty() { + return; } - for ch in &sh.charts { - let r = anchor_rect(sh, &ch.anchor); - if !visible(r) { - continue; - } - let (x, y, _, _) = place(r); - let data = gridcraft_chart::resolve(wb, si, ch); - let prims = gridcraft_chart::render(ch, &data, r.2 as f32, r.3 as f32, &PdfMeasure); - draw_prims(pg, &prims, Xf { ox: x, oy: y, k: b.k }); + use gridcraft_model::{ShapeTextAlign, ShapeTextAnchor}; + let fm = &shp.format; + let text_box = shp.kind == ShapeKind::TextBox; + let size = fm.font_size.unwrap_or(11.0).clamp(1.0, 4000.0) * b.scale; + let font = Font::styled(Font::family_for(fm.font.as_deref().unwrap_or("")), fm.bold, fm.italic); + let color = fm.text_color.and_then(|c| rgb_of(c, wb)).unwrap_or([0, 0, 0]); + let align = fm.align.unwrap_or(if text_box { ShapeTextAlign::Left } else { ShapeTextAlign::Center }); + let anchor = fm.anchor.unwrap_or(if text_box { ShapeTextAnchor::Top } else { ShapeTextAnchor::Middle }); + let lines = wrap_lines(&shp.text, font, size, (w - 4.0 * b.k).max(size)); + let block_h = lines.len().min(500) as f32 * size * 1.2; + let mut yy = match anchor { + ShapeTextAnchor::Top => y + 2.0 * b.k, + ShapeTextAnchor::Middle => y + (h - block_h) / 2.0, + ShapeTextAnchor::Bottom => y + h - 2.0 * b.k - block_h, + } + size * 0.8; + for l in lines.iter().take(500) { + let lw = gridcraft_pdf::text_width(font, size, l); + let lx = match align { + ShapeTextAlign::Left => x + 2.0 * b.k, + ShapeTextAlign::Center => x + (w - lw) / 2.0, + ShapeTextAlign::Right => x + w - 2.0 * b.k - lw, + }; + pg.text(lx, yy, size, font, color, l); + yy += size * 1.2; } - pg.restore(); } fn draw_headings(pg: &mut Page, b: &Block, col_heads: bool, row_heads: bool, at: (f32, f32), size: (f32, f32)) { @@ -1528,6 +1552,9 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str let mut page_no = 0usize; let mut pending_links = Vec::new(); let mut printed_blocks = Vec::new(); + // Decoded pictures are reused across the pages their anchor covers: a floating picture + // spanning 40 pages was decoded (and pushed into the PDF) 40 times (#2389). + let mut pics: std::collections::HashMap<(usize, usize), Option<(Vec, u32, u32)>> = std::collections::HashMap::new(); for lay in layouts { let Some(sh) = wb.sheet(lay.sheet) else { continue }; let mut cf = if sh.cond_formats.is_empty() { None } else { Some(crate::cf::CfCache::new()) }; @@ -1564,7 +1591,7 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str y += seg_size(sh, *rs, true) * k; } if let Some(b) = &body { - draw_objects(pg, wb, lay.sheet, sh, b); + draw_objects(pg, wb, lay.sheet, sh, b, &mut pics); } if lay.settings.headings { let fill = [0xF2, 0xF2, 0xF2]; diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index a453f250b..e6caa1dc7 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -401,8 +401,10 @@ fn move_columns(s: &mut Session, p: &Json) -> Result { { return Err(EngineError::Other("To prevent possible loss of data, columns cannot be shifted off of the worksheet.".into())); } - check_lines_protection(s, &json!({"cols": RangeRef::cols(before, before).a1()}), Axis::Cols, before, count, true)?; - check_lines_protection(s, &json!({"cols": RangeRef::cols(from, end - 1).a1()}), Axis::Cols, from, count, false)?; + // The checks must authorize the sheet this command modifies (`target_sheet`), not whatever + // sheet is active: both carry the command's own `sheet` parameter on. + check_lines_protection(s, &json!({"cols": RangeRef::cols(before, before).a1(), "sheet": p.get("sheet")}), Axis::Cols, before, count, true)?; + check_lines_protection(s, &json!({"cols": RangeRef::cols(from, end - 1).a1(), "sheet": p.get("sheet")}), Axis::Cols, from, count, false)?; let moved_from = if before <= from { from + count } else { from }; if moved_from.checked_add(count).is_none_or(|end| end > MAX_COLS) { diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index b18e8577b..be47e0a67 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -448,6 +448,11 @@ impl std::fmt::Write for ClipboardHtml { /// Most rows Save as Web Page writes. A sheet whose used range is taller fails instead of losing cells. const HTML_EXPORT_MAX_ROWS: u32 = 100_001; +/// Rows × columns of one web-page table. Save as Web Page writes every kept row's full used +/// column span into one growing String, so a sparse sheet with a lone far-right cell (or a +/// tall, wide one) would otherwise be gigabytes of `` before finishing (#2387). +const HTML_EXPORT_MAX_CELLS: u64 = 4_000_000; + /// A sheet as a standalone HTML table (Save as Web Page). A sheet with pictures gets the sheet's /// own column widths and row heights so each picture and chart (inline SVG) can sit at its anchor. pub fn to_html(wb: &Workbook, sheet: usize) -> Result { @@ -479,6 +484,17 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result { r.height() ))); } + // The table is one growing String: every kept row writes a `` for the whole used + // column span (unless the sheet has no row that is that wide). A tall, wide sheet + // (or a sparse one with a lone far-right cell) would be gigabytes before finishing, + // so the rectangle itself is bounded (#2387). + if u64::from(r.height()) * u64::from(r.width()) > HTML_EXPORT_MAX_CELLS { + return Err(EngineError::Other(format!( + "the sheet is too tall × wide for a web page ({} × {} cells, at most {HTML_EXPORT_MAX_CELLS}). Save as .xlsx or CSV instead.", + r.height(), + r.width() + ))); + } if fixed { out.push_str(""); for col in (r.start.col..=r.end.col).filter(|&c| !sh.is_col_hidden(c)) { @@ -588,14 +604,27 @@ fn script_sandbox_check(path: &str) -> Result<()> { Ok(()) } +/// The largest native file the reader still touches: its own cap plus one, used to stop +/// reads whose bytes arrive faster than metadata can claim (device files, growing files). +const MAX_READ_BYTES: u64 = 2 * 1024 * 1024 * 1024; + #[cfg(not(target_arch = "wasm32"))] pub fn read_file(path: &str) -> Result> { + use std::io::Read; script_sandbox_check(path)?; - let meta = std::fs::metadata(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; - if meta.len() > 2 * 1024 * 1024 * 1024 { + let f = std::fs::File::open(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + // A symlink to an endless device reports length 0 but streams forever, so metadata + // alone never bounds the read: require a real file and cap the bytes actually taken. + if !f.metadata().map_err(|e| EngineError::Other(format!("{path}: {e}")))?.is_file() { + return Err(EngineError::Other(format!("{path}: only regular files can be opened"))); + } + let mut buf = Vec::new(); + let mut limited = (&f).take(MAX_READ_BYTES + 1); + limited.read_to_end(&mut buf).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + if buf.len() as u64 > MAX_READ_BYTES { return Err(EngineError::Other("files larger than 2 GB aren't supported".into())); } - std::fs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}"))) + Ok(buf) } #[cfg(target_arch = "wasm32")] @@ -606,10 +635,35 @@ pub fn read_file(path: &str) -> Result> { #[cfg(not(target_arch = "wasm32"))] pub fn write_file(path: &str, bytes: &[u8]) -> Result<()> { + use std::io::Write; script_sandbox_check(path)?; // Write to a temporary file, then rename: a crash never leaves a half-written workbook. + // The temporary file is created exclusively, so an existing file or symlink at that + // path (planted by another local user) makes the save fail instead of truncating the + // link's target; and it inherits the destination's access restrictions, so saving a + // private workbook doesn't widen who can read it. let tmp = format!("{path}.sctmp"); - std::fs::write(&tmp, bytes).and_then(|()| std::fs::rename(&tmp, path)).map_err(|e| { + let prior = std::fs::metadata(path).ok().map(|m| m.permissions()); + let mut opts = std::fs::OpenOptions::new(); + opts.write(true).create_new(true); + #[cfg(unix)] + if let Some(p) = &prior { + use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + opts.mode(p.mode()); + } + let mut f = opts.open(&tmp).map_err(|e| { + let _ = std::fs::remove_file(&tmp); + EngineError::Other(format!("{path}: {e}")) + })?; + #[cfg(not(unix))] + if let Some(p) = &prior { + let _ = f.set_permissions(p.clone()); + } + f.write_all(bytes).and_then(|()| f.flush()).map_err(|e| { + let _ = std::fs::remove_file(&tmp); + EngineError::Other(format!("{path}: {e}")) + })?; + std::fs::rename(&tmp, path).map_err(|e| { // A write or rename that fails (full disk, the workbook locked by another app) leaves // no temporary file beside the user's workbook. let _ = std::fs::remove_file(&tmp); @@ -711,6 +765,74 @@ mod tests { range_html(wb, 0, r, &crate::display::range_text(wb, 0, r)).unwrap() } + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn save_fails_upfront_when_the_temp_name_is_taken() { + // Another local user who can write the directory must not be able to point the + // save's predictable temporary name at their own file: creation is exclusive. + let dir = std::env::temp_dir().join("gridcraft-fs-safety"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let victim = dir.join("victim.txt"); + std::fs::write(&victim, b"original").unwrap(); + let dest = dir.join("book.xlsx"); + std::os::unix::fs::symlink(&victim, dir.join("book.xlsx.sctmp")).unwrap(); + let err = write_file(dest.to_str().unwrap(), b"new").unwrap_err(); + assert_eq!(std::fs::read(&victim).unwrap(), b"original"); + assert!(!dir.join("book.xlsx").exists(), "{err}"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[cfg(not(target_arch = "wasm32"))] + #[cfg(unix)] + #[test] + fn save_keeps_the_destination_access_restrictions() { + use std::os::unix::fs::PermissionsExt; + let dir = std::env::temp_dir().join("gridcraft-fs-safety"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let dest = dir.join("private.xlsx"); + std::fs::write(&dest, b"v1").unwrap(); + std::fs::set_permissions(&dest, std::fs::Permissions::from_mode(0o600)).unwrap(); + write_file(dest.to_str().unwrap(), b"v2").unwrap(); + let mode = std::fs::metadata(&dest).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + // And a brand-new file is created with the restrictive, not permissive, default. + let fresh = dir.join("new.xlsx"); + write_file(fresh.to_str().unwrap(), b"v").unwrap(); + let mode = std::fs::metadata(&fresh).unwrap().permissions().mode() & 0o777; + assert!(mode & 0o022 == 0, "mode {mode:o}"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn open_refuses_directories_and_unknowable_lengths() { + let dir = std::env::temp_dir().join("gridcraft-fs-safety"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + assert!(read_file(dir.to_str().unwrap()).is_err(), "a directory is not a regular file"); + assert!(read_file(dir.join("missing").to_str().unwrap()).is_err()); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn html_export_refuses_an_outsized_grid_instead_of_growing_forever() { + // #2387: the used rectangle alone bounds the output (every row writes the whole + // column span), so a sparse sheet with one cell at A1 and another far right + // 300 rows down is 4.9M cells → refused before a byte of `` is written. + let mut wb = Workbook::new(); + let sh = wb.sheet_mut(0).unwrap(); + sh.set_value(CellRef::new(0, 0), gridcraft_core::Value::Text("start".into())); + sh.set_value(CellRef::new(300, 16383), gridcraft_core::Value::Text("corner".into())); + let err = to_html(&wb, 0).unwrap_err(); + assert!(err.to_string().contains("too tall × wide"), "{err}"); + // A normal sheet is fine. + let mut ok = Workbook::new(); + ok.sheet_mut(0).unwrap().set_value(CellRef::new(5, 2), gridcraft_core::Value::Text("t".into())); + assert!(to_html(&ok, 0).is_ok()); + } + #[test] fn html_export_embeds_images_as_data_uris() { let mut wb = Workbook::new(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 1be90d56c..f4c47bb62 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4604,4 +4604,60 @@ fn html_export_without_pictures_or_charts_has_no_fixed_geometry() { let html = crate::io::to_html(&s.doc().unwrap().wb, 0).unwrap(); assert!(!html.contains("") && !html.contains("table-layout") && !html.contains(" = { + let d = s.doc().unwrap(); + let sh = &d.wb.sheets[d.wb.sheet_index("P").unwrap()]; + (0..4).map(|c| sh.value(CellRef::new(0, c as u32))).collect() + }; + assert_eq!(pv, vec![Value::number(1.0), Value::number(2.0), Value::number(3.0), Value::number(4.0)]); + // The active, unprotected sheet moves fine. + s.execute("range.setValues", json!({"range": "A1", "values": [["A", "B", "C", "D"]]})).unwrap(); + s.execute("sheet.moveColumns", json!({"cols": "B:B", "before": 4})).unwrap(); + assert_eq!(v(&s, "A1"), Value::text("A")); +} + +#[test] +fn a_script_cannot_switch_windows() { + // Stored scripts are scoped to their workbook: a step that activates another window would + // point every later step at another open document, so it is refused before any step runs. + let mut s = s(); + s.execute("automate.newScript", json!({"name": "t", "commands": [{"command": "window.activate", "params": {"index": 0}}]})).unwrap(); + s.execute("automate.trustScripts", json!({"trusted": true})).unwrap(); + let err = s.execute("automate.runScript", json!({"name": "t"})).expect_err("script switching windows"); + assert!(err.to_string().contains("window.activate"), "{err}"); + // A benign script runs. + s.execute("automate.newScript", json!({"name": "ok", "commands": [{"command": "cell.set", "params": {"cell": "A1", "input": "=1+1"}}]})).unwrap(); + s.execute("automate.runScript", json!({"name": "ok"})).unwrap(); + assert_eq!(v(&s, "A1"), Value::number(2.0)); +} + mod seams; diff --git a/crates/functions/src/criteria.rs b/crates/functions/src/criteria.rs index 58f0e6594..9aa539922 100644 --- a/crates/functions/src/criteria.rs +++ b/crates/functions/src/criteria.rs @@ -264,15 +264,27 @@ pub(crate) fn has_wildcards(pattern: &str) -> bool { pub(crate) fn wildcard_match(pattern: &str, text: &str) -> bool { let p = tokenize(pattern); let t = lower_chars(text); - match_from(&p, &t, true).is_some() + let mut budget = MAX_STEPS; + match_from(&p, &t, true, &mut budget).is_some() } +/// Steps one `match_from` call may take. An honest match against Excel's 32,767-character +/// cell ceiling costs a few hundred thousand; a hostile one (star churn) is capped instead +/// of quadratic — a call that spends more gives up and reports no match, quiet degradation +/// like the other work caps on far-past-realistic input. +const MAX_STEPS: u64 = 1 << 26; + /// Greedy-free matcher: returns the matched length of `t` when `p` matches a prefix of `t` -/// (or all of `t` when `whole`). Iterative with backtracking on the last star (linear-ish). -fn match_from(p: &[Tok], t: &[char], whole: bool) -> Option { +/// (or all of `t` when `whole`). Iterative with backtracking on the last star (linear-ish, +/// quadratic at worst). `(budget, whole)`: the budget is spent one per loop step; a call that +/// would spend more than it has gives up and reports no match — quiet degradation, as with the +/// other work caps on far-past-realistic input (an honest match against Excel's 32,767-char +/// cell ceiling costs a few hundred thousand steps). +fn match_from(p: &[Tok], t: &[char], whole: bool, budget: &mut u64) -> Option { let (mut pi, mut ti) = (0usize, 0usize); let mut star: Option<(usize, usize)> = None; loop { + *budget = budget.checked_sub(1)?; if pi == p.len() && (!whole || ti == t.len()) { return Some(ti); } @@ -307,10 +319,32 @@ fn match_from(p: &[Tok], t: &[char], whole: bool) -> Option { pub(crate) fn wildcard_find(pattern: &str, text: &str, start: usize) -> Option { let p = tokenize(pattern); let t = lower_chars(text); + // A leading run of stars never matters for a prefix match (it consumes nothing), so + // dropping it lets the anchor below skip linearly through the text (#2382). + let skip = p.iter().take_while(|tk| matches!(tk, Tok::Star)).count(); + let p = &p[skip.min(p.len())..]; if p.is_empty() { return if start <= t.len() { Some(start) } else { None }; } - (start..t.len()).find(|&i| t.get(i..).is_some_and(|rest| match_from(&p, rest, false).is_some())) + // The first token that consumes a character anchors the scan: a literal skips straight + // to its occurrences, and after leading `?`s each attempt only pays for the `?` run and + // one suffix try. Every attempt shares one budget, so the total work stays flat. + let anys = p.iter().take_while(|tk| matches!(tk, Tok::Any)).count(); + let anchor = p.get(anys); + let mut budget = MAX_STEPS; + let mut i = start; + while i < t.len() { + let hit = match (anchor, t.get(i + anys)) { + (Some(Tok::Char(c)), Some(rest)) => *rest == *c && match_from(p, &t[i..], false, &mut budget).is_some(), + (Some(Tok::Char(_)), None) => break, // the anchor char no longer exists ahead + _ => match_from(p, &t[i..], false, &mut budget).is_some(), + }; + if hit { + return Some(i); + } + i += 1; + } + None } /// Exact-or-wildcard equality used by MATCH/VLOOKUP/XLOOKUP exact modes: text with wildcards diff --git a/crates/ui-egui/src/floating_pictures.rs b/crates/ui-egui/src/floating_pictures.rs index 68de6e203..5caa0869c 100644 --- a/crates/ui-egui/src/floating_pictures.rs +++ b/crates/ui-egui/src/floating_pictures.rs @@ -95,6 +95,13 @@ impl Cache { } None => (None, u32::MAX), }; + // Eviction only frees entries used before this frame, so a workbook covered in same-frame + // pictures can leave the cache unable to make room. Then draw the texture without + // retaining it: the handle is dropped at the end of the frame and the GPU memory goes + // with it, instead of the budget being outflanked (finding: cache admission). + if self.entries.len() >= MAX_CACHE_ITEMS || self.bytes.saturating_add(bytes) > MAX_CACHE_BYTES { + return texture; + } self.bytes = self.bytes.saturating_add(bytes); self.entries.insert(key, Entry { texture: texture.clone(), bytes, side, used: frame }); texture @@ -212,13 +219,24 @@ mod tests { } assert!(cache.bytes <= MAX_CACHE_BYTES, "{} bytes kept", cache.bytes); assert!(cache.entries.len() < data.len()); - // Many on screen at once, over the budget: every one keeps its texture, frame after frame. + // Many on screen at once, over the budget: each still gets a texture this frame, but the + // cache cannot grow past its budget whatever the workbook shows. Eight of the 16 MB + // textures fit the 128 MB budget, and those stay pinned; the rest re-upload each frame + // instead of being retained over it (finding: cache admission). + // Two frames pass so the rehearsal's entries are old enough to be evictable. + frame(&ctx); + frame(&ctx); let shown: Vec<_> = data.iter().enumerate().take(12).map(|(i, d)| cache.texture(&ctx, key(1, i as u32, d), d, 2048).unwrap().id()).collect(); for _ in 0..3 { frame(&ctx); for (i, d) in data.iter().enumerate().take(12) { - assert_eq!(cache.texture(&ctx, key(1, i as u32, d), d, 2048).map(|t| t.id()), Some(shown[i])); + let id = cache.texture(&ctx, key(1, i as u32, d), d, 2048).unwrap().id(); + if i < 8 { + assert_eq!(id, shown[i], "a picture that fits keeps its texture"); + } } + assert!(cache.bytes <= MAX_CACHE_BYTES, "{} bytes kept", cache.bytes); + assert!(cache.entries.len() <= 8, "{} entries kept", cache.entries.len()); } } diff --git a/crates/xlsx/src/crypto.rs b/crates/xlsx/src/crypto.rs index 46a19d046..0a63044a0 100644 --- a/crates/xlsx/src/crypto.rs +++ b/crates/xlsx/src/crypto.rs @@ -26,6 +26,9 @@ const BLOCK: usize = 16; pub(crate) const BLOCK_VERIFIER_INPUT: [u8; 8] = [0xFE, 0xA7, 0xD2, 0x76, 0x3B, 0x4B, 0x9E, 0x79]; pub(crate) const BLOCK_VERIFIER_HASH: [u8; 8] = [0xD7, 0xAA, 0x0F, 0x6D, 0x30, 0x61, 0x34, 0x4E]; pub(crate) const BLOCK_KEY: [u8; 8] = [0x14, 0x6E, 0x0B, 0xE7, 0xAB, 0xAC, 0xD0, 0xD6]; +/// The two dataIntegrity block keys ([MS-OFFCRYPTO] 2.3.4.14), one per HMAC field. +pub(crate) const BLOCK_HMAC_KEY: [u8; 8] = [0x5F, 0xB2, 0xAD, 0x01, 0x0C, 0xB9, 0xE1, 0xF6]; +pub(crate) const BLOCK_HMAC_VALUE: [u8; 8] = [0xA0, 0x67, 0x7F, 0x02, 0xB2, 0x2C, 0x84, 0x33]; /// The password key encryptor's URI (the other kind is certificate-based). pub(crate) const PASSWORD_ENCRYPTOR: &str = "http://schemas.microsoft.com/office/2006/keyEncryptor/password"; @@ -165,6 +168,14 @@ impl Hash { Hash::Sha512 => run::(parts), } } + + /// The bytes the algorithm's compression function takes: 128 for SHA-384/512, else 64. + fn block_size(self) -> usize { + match self { + Hash::Sha384 | Hash::Sha512 => 128, + Hash::Sha1 | Hash::Sha256 => 64, + } + } } /// H0 = H(salt ‖ UTF-16LE(password)), then Hn = H(LE32(n - 1) ‖ Hn-1) for `spin` rounds. @@ -184,6 +195,19 @@ pub(crate) fn derive(hash: Hash, a: &[u8], b: &[u8], len: usize) -> Vec { k } +/// HMAC-SHA-`hash` ([RFC2104]), as Agile's dataIntegrity uses. +pub(crate) fn hmac(hash: Hash, key: &[u8], message: &[u8]) -> Vec { + let bs = hash.block_size(); + let mut k = key.to_vec(); + if k.len() > bs { + k = hash.digest(&[&k]); + } + k.resize(bs, 0); + let pad = |x: u8| k.iter().map(|b| b ^ x).collect::>(); + let inner = hash.digest(&[&pad(0x36), message]); + hash.digest(&[&pad(0x5C), &inner]) +} + /// The Standard encryption key ([MS-OFFCRYPTO] 2.3.4.7): SHA-1 iterated 50,000 times, the /// block-0 hash, then the 0x36 / 0x5C expansion, truncated to `key_len`. pub(crate) fn standard_key(password: &str, salt: &[u8], key_len: usize) -> Vec { @@ -368,8 +392,9 @@ impl Params { } } -/// What an Agile descriptor holds: the package's cipher, and the password key encryptor with its -/// three encrypted values (verifier input, verifier hash, package key). +/// What an Agile descriptor holds: the package's cipher, the password key encryptor with its +/// three encrypted values (verifier input, verifier hash, package key), and the package's +/// integrity pair, which Excel always writes and the reader requires. struct AgileDescriptor { key_data: Params, key: Params, @@ -377,6 +402,8 @@ struct AgileDescriptor { verifier_input: Vec, verifier_hash: Vec, key_value: Vec, + hmac_key: Vec, + hmac_value: Vec, } /// `xml` is the XML encryption descriptor after the 8-byte version and reserved field. @@ -395,6 +422,16 @@ fn agile_descriptor(xml: &[u8]) -> Result { return Err(malformed(format!("a spin count of {spin}, more than the {MAX_SPIN_COUNT} allowed"))); } let field = |name: &str| base64(encryptor.attr(name).ok_or_else(|| malformed(format!("no {name}")))?); + // A descriptor without dataIntegrity can't prove its ciphertext is the one the password + // protects: the verifier only authenticates the key, not the package ([MS-OFFCRYPTO] 2.3.4.14). + let integrity = root.child("dataIntegrity").ok_or_else(|| malformed("no dataIntegrity"))?; + let hmac_field = |attr: &str| { + let v = base64(integrity.attr(attr).ok_or_else(|| malformed(format!("no {attr}")))?)?; + if v.is_empty() || v.len() % BLOCK != 0 { + return Err(malformed(format!("a whole-block {attr}"))); + } + Ok(v) + }; Ok(AgileDescriptor { key_data, key, @@ -402,11 +439,14 @@ fn agile_descriptor(xml: &[u8]) -> Result { verifier_input: field("encryptedVerifierHashInput")?, verifier_hash: field("encryptedVerifierHashValue")?, key_value: field("encryptedKeyValue")?, + hmac_key: hmac_field("encryptedHmacKey")?, + hmac_value: hmac_field("encryptedHmacValue")?, }) } fn agile(xml: &[u8], package: &[u8], password: &str) -> Result, DecryptError> { - let AgileDescriptor { key_data, key, spin, verifier_input, verifier_hash, key_value } = agile_descriptor(xml)?; + let descriptor = agile_descriptor(xml)?; + let AgileDescriptor { key_data, key, spin, verifier_input, verifier_hash, key_value, hmac_key, hmac_value } = descriptor; let h = iterated_hash(key.hash, password, &key.salt, spin); let iv = key.iv(None); let field = |data: &[u8], block_key: &[u8]| -> Result, DecryptError> { @@ -423,6 +463,18 @@ fn agile(xml: &[u8], package: &[u8], password: &str) -> Result, DecryptE } let secret = field(&key_value, &BLOCK_KEY)?; let aes = Aes::new(secret.get(..key_data.key_len).ok_or_else(|| malformed("a short encryptedKeyValue"))?)?; + // Package integrity, [MS-OFFCRYPTO] 2.3.4.14: both HMAC fields are encrypted with the + // package key (the verifier above only authenticates the password), keyed by a random + // salt-size array, computed over the whole EncryptedPackage stream, StreamSize field included. + let mut mac_field = hmac_key; + aes.decrypt_cbc(&key_data.iv(Some(&BLOCK_HMAC_KEY)), &mut mac_field)?; + mac_field.truncate(key_data.salt.len()); + let mut declared = hmac_value; + aes.decrypt_cbc(&key_data.iv(Some(&BLOCK_HMAC_VALUE)), &mut declared)?; + let mac = hmac(key_data.hash, &mac_field, package); + if declared.get(..mac.len()) != Some(mac.as_slice()) { + return Err(malformed("the decrypted data failed its integrity check")); + } let (size, data) = package_parts(package)?; let mut out = Vec::with_capacity(size); for (i, segment) in data.chunks(SEGMENT).enumerate() { diff --git a/crates/xlsx/src/crypto/tests.rs b/crates/xlsx/src/crypto/tests.rs index 0fb0b921b..5f14686e9 100644 --- a/crates/xlsx/src/crypto/tests.rs +++ b/crates/xlsx/src/crypto/tests.rs @@ -6,6 +6,9 @@ use super::*; use crate::testing::{agile_info, encrypt_agile, encrypt_standard, with_agile_header}; const PASSWORD: &str = "Pass1234"; +/// 16 zero bytes (whole AES blocks), a stand-in for a real integrity field where the values +/// can't be reached anyway (scheme errors, descriptor-parse damage). +const HMAC_STUB: &str = "AAAAAAAAAAAAAAAAAAAAAA=="; fn hex(s: &str) -> Vec { (0..s.len()).step_by(2).map(|i| u8::from_str_radix(&s[i..i + 2], 16).unwrap()).collect() @@ -107,9 +110,9 @@ fn unsupported_schemes_are_named() { assert_eq!(unsupported(&rc4), "RC4"); assert_eq!(unsupported(&[3, 0, 3, 0, 0, 0, 0, 0]), "Extensible"); assert_eq!(unsupported(&[1, 0, 1, 0, 0, 0, 0, 0]), "version 1.1"); - let des = agile_info(PASSWORD, 1000).replace("cipherAlgorithm=\"AES\"", "cipherAlgorithm=\"3DES\""); + let des = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace("cipherAlgorithm=\"AES\"", "cipherAlgorithm=\"3DES\""); assert_eq!(unsupported(&with_agile_header(&des)), "3DES"); - let cert = agile_info(PASSWORD, 1000).replace("keyEncryptor/password", "keyEncryptor/certificate"); + let cert = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace("keyEncryptor/password", "keyEncryptor/certificate"); assert_eq!(unsupported(&with_agile_header(&cert)), "certificate"); } @@ -136,7 +139,7 @@ fn damaged_files_are_malformed_not_panics() { ragged[..8].copy_from_slice(&len.to_le_bytes()); assert!(is_malformed(decrypt_streams(&info, &ragged, PASSWORD))); } - let xml = agile_info(PASSWORD, 1000); + let xml = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB); let bad = |s: String| is_malformed(decrypt_streams(&with_agile_header(&s), &[0; 8], PASSWORD)); assert!(bad(xml.replace("spinCount=\"1000\"", "spinCount=\"4000000000\""))); assert!(bad(xml.replace("saltValue=\"", "saltValue=\"*"))); @@ -155,3 +158,76 @@ fn non_ascii_passwords_hash_as_utf16le_like_msoffcrypto() { hex("2bca96ca0a7e8d88d9314a2eb1819a1ece94ff9ce34266ff82f891a88c4240c4c9f39c729b7c5c410c27fc8e2b7ad90d8c005eb68a603377b8a4ab858041c76a") ); } + +#[test] +fn hmac_matches_the_rfc2104_test_vectors() { + use crate::crypto::hmac as mac; + let h = |v: Vec| v.iter().map(|b| format!("{b:02x}")).collect::(); + // RFC 2202 test case 1 (HMAC-SHA-1) and HMAC-SHA-256 vectors. + assert_eq!(h(mac(Hash::Sha1, &[0x0b; 20], b"Hi There")), "b617318655057264e28bc0b6fb378c8ef146be00"); + assert_eq!(h(mac(Hash::Sha256, &[0x0b; 20], b"Hi There")), "b0344c61d8db38535ca8afceaf0bf12b881dc200c9833da726e9376c2e32cff7"); + // RFC 4231 test case 5 (HMAC-SHA-256): a key longer than the block size is hashed first. + assert_eq!( + h(mac(Hash::Sha256, &[0xaa; 131], b"Test Using Larger Than Block-Size Key - Hash Key First")), + "60e431591ee0b67f0d8a26aacbf5b77f8e0bc6213728c5140546040f0ee37f54" + ); +} + +/// Strips the `` element from an Agile descriptor. +fn without_integrity(info: &[u8]) -> Vec { + let xml = std::str::from_utf8(&info[8..]).unwrap().to_string(); + let start = xml.find("").unwrap() + 2; + let stripped = format!("{}{}", &xml[..start], &xml[end..]); + with_agile_header(&stripped) +} + +#[test] +fn integrity_is_required_and_enforced() { + let plain = payload(300); + let (info, package) = encrypt_agile(&plain, PASSWORD, 1000); + // An attacker can't just delete the integrity element to get the package verified. + assert!(matches!(decrypt_streams(&without_integrity(&info), &package, PASSWORD), + Err(DecryptError::Malformed(m)) if m.contains("dataIntegrity"))); + // An empty or ragged field is damage, not a pass. + for bad in [("encryptedHmacKey", ""), ("encryptedHmacKey", "AA=="), ("encryptedHmacValue", "")] { + let xml = + agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace(&format!("{}=\"{HMAC_STUB}\"", bad.0), &format!("{}=\"{}\"", bad.0, bad.1)); + let r = decrypt_streams(&with_agile_header(&xml), &package, PASSWORD); + assert!(matches!(r, Err(DecryptError::Malformed(_))), "{}=\"{}\"", bad.0, bad.1); + } +} + +#[test] +fn modified_ciphertext_fails_the_integrity_check() { + let plain = payload(300); + let (info, package) = encrypt_agile(&plain, PASSWORD, 1000); + // The correct password proves the verifier can't vouch for a modified package. + for at in [8usize, 100, package.len() - 5] { + let mut tampered = package.clone(); + tampered[at] ^= 0x80; + assert!( + matches!(decrypt_streams(&info, &tampered, PASSWORD), + Err(DecryptError::Malformed(m)) if m.contains("integrity")), + "byte {at}" + ); + } + // And neither can the descriptor: a swapped integrity value is refused. + let xml = String::from_utf8_lossy(&info[8..]).to_string(); + let at = xml.find("encryptedHmacValue=\"").unwrap() + "encryptedHmacValue=\"".len(); + let mut xml: Vec = xml.into_bytes(); + xml[at] ^= b'A' ^ b'B'; + assert!(matches!(decrypt_streams(&with_agile_header(std::str::from_utf8(&xml).unwrap()), &package, PASSWORD), Err(DecryptError::Malformed(_)))); +} + +#[test] +fn integrity_matches_the_stream_not_the_plaintext() { + // The MAC covers the whole EncryptedPackage stream, StreamSize field included: changing the + // stated size alone must fail, while the actual bytes round-trip. + let plain = payload(4200); + let (info, package) = encrypt_agile(&plain, PASSWORD, 1000); + let mut ours = package.clone(); + ours[..8].copy_from_slice(&((plain.len() + 1) as u64).to_le_bytes()); + assert!(matches!(decrypt_streams(&info, &ours, PASSWORD), Err(DecryptError::Malformed(m)) if m.contains("integrity"))); + assert_eq!(decrypt_streams(&info, &package, PASSWORD).unwrap(), plain); +} diff --git a/crates/xlsx/src/package.rs b/crates/xlsx/src/package.rs index e5bde555f..a6950a07e 100644 --- a/crates/xlsx/src/package.rs +++ b/crates/xlsx/src/package.rs @@ -74,14 +74,24 @@ impl<'a> Package<'a> { let remaining = MAX_TOTAL.saturating_sub(self.total); let limit = maximum.min(remaining); let mut out = Vec::with_capacity(f.size().min(16 * 1024 * 1024) as usize); - (&mut f).take(limit + 1).read_to_end(&mut out).map_err(|e| IoError::Zip(format!("{part}: {e}")))?; + let read = (&mut f).take(limit + 1).read_to_end(&mut out).map_err(|e| { + // A read that fails partway has already spent decompression work and buffered data + // the package budget never sees: count the allowance as spent, so a loop over many + // broken parts can't decompress it over and over. + self.total = MAX_TOTAL; + IoError::Zip(format!("{part}: {e}")) + }); if out.len() as u64 > limit { + // The allowance was spent getting here; make the exhaustion terminal instead of + // letting later reads pay the same decompression cost again. + self.total = MAX_TOTAL; return Err(IoError::TooLarge(if remaining < maximum { "package expands to more than 1 GB".into() } else { format!("{part} expands to more than {maximum} bytes") })); } + read?; self.total += out.len() as u64; Ok(Some(out)) } @@ -201,4 +211,20 @@ mod tests { assert_eq!(split_dir("xl/workbook.xml"), ("xl", "workbook.xml")); assert_eq!(split_dir("x.xml"), ("", "x.xml")); } + + /// An over-budget read must spend the package's allowance once and for all: loops over many + /// broken parts (a sheet's optional VML relationships, for one) otherwise re-decompress the + /// remaining allowance again and again. + #[test] + fn a_failed_part_ends_the_package_budget() { + let z = crate::tests::make_zip(&[("a.xml", b"0123456789"), ("b.xml", b"hello")]); + let mut pkg = Package::open(&z).unwrap(); + assert!(pkg.read("a.xml").unwrap().is_some()); + // Nothing left to spend within the per-part limit either. + pkg.total = MAX_TOTAL - 1; + assert!(pkg.read("b.xml").is_err()); + assert_eq!(pkg.total, MAX_TOTAL, "the failed read is charged"); + // Oversized repeated attempts stay cheap errors now: no allowance is left to pay for. + assert!(matches!(pkg.read("b.xml"), Err(IoError::TooLarge(_)))); + } } diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 5af3aeab3..7cb2e8c22 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -33,12 +33,62 @@ struct RowState { /// Dynamic-array spill ranges and legacy array ranges: cached values inside them (other than /// the anchor) are not stored, so the formula can fill them again. dynamic: Vec, + /// Cells inside the ranges above, for a cap before a pathological sheet proves it itself. + dynamic_cells: u64, + /// Row-membership checks served so far, for the work cap above. + dynamic_checks: u64, dropped: u64, rows: u64, } +/// Whether `pos` lies in a known dynamic range that isn't its anchor: the cell's cached value +/// gives way to the formula that will fill it. Charges every scanned range against one work +/// budget, then gives up consulting the ranges at all: the worst kind of file may then import +/// a few cached spill values, but it never stalls the whole import. +fn in_dynamic(st: &mut RowState, pos: CellRef) -> bool { + if st.dynamic_checks > MAX_DYNAMIC_CHECKS || st.dynamic.is_empty() { + return false; + } + let ranges = st.dynamic.len(); + let hit = st.dynamic.iter().any(|r| r.contains(pos) && r.start != pos); + st.dynamic_checks = st.dynamic_checks.saturating_add(ranges as u64); + hit +} + +/// Records a dynamic-array range, so its cells outside the anchor import as empty and the +/// formula fills them again. Repeated identical anchors (a damaged or hostile sheet may repeat +/// the same record) are recorded once, and count and area are capped: every later cell inside +/// known range territory pays a scan proportional to what is recorded here. +fn push_dynamic(st: &mut RowState, range: RangeRef) { + if st.dynamic.contains(&range) { + return; + } + let cells = range.count(); + if st.dynamic.len() >= MAX_DYNAMIC_RANGES || st.dynamic_cells.saturating_add(cells) > MAX_DYNAMIC_CELLS { + return; + } + st.dynamic_cells = st.dynamic_cells.saturating_add(cells); + st.dynamic.push(range); +} + const MAX_HYPERLINK_CELLS: u64 = 10_000; +/// Target and tooltip bytes kept for hyperlinks of one sheet, over all cells: the same URL is +/// copied for every cell of its range, so length × cells must stay bounded whatever the file. +const MAX_HYPERLINK_BYTES: u64 = 8 * 1024 * 1024; const MAX_COLUMN_ASSIGNMENTS: u64 = 1_000_000; +/// Most dynamic-array and legacy-array ranges of one sheet. +const MAX_DYNAMIC_RANGES: usize = 65_536; +/// Worst-case cells inside the dynamic ranges the sheet declares. Beyond it further ranges are +/// dropped: they would cost their storage in every membership lookup that follows. +const MAX_DYNAMIC_CELLS: u64 = 16_000_000; +/// Most row-membership checks the sheet's ranges may serve. Beyond it the remaining cells are +/// imported with their cached values instead of being blanked for a spill that may still come, +/// so adversarial range counts can't make each cell scan a long list (a real workbook has a +/// handful of ranges). +const MAX_DYNAMIC_CHECKS: u64 = 16_000_000; +/// Most conditional-format range cells one sheet keeps, over all rules of an element: every +/// rule copies its element's whole range list, so rules × ranges must stay bounded. +const MAX_COND_CELLS: u64 = 4_000_000; pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result { let bytes = cx.pkg.read(part)?.ok_or_else(|| IoError::Format(format!("missing part {part}")))?; @@ -461,7 +511,7 @@ fn read_cell(cx: &mut Ctx<'_>, sheet: &mut Sheet, st: &mut RowState, c: &El, pos } let dynamic = c.attr("cm").is_some(); let formula = c.child("f").and_then(|f| read_formula(cx, st, f, pos, dynamic)); - let value = if formula.is_none() && st.dynamic.iter().any(|r| r.contains(pos) && r.start != pos) { Value::Empty } else { value }; + let value = if formula.is_none() && in_dynamic(st, pos) { Value::Empty } else { value }; sheet.cells.set(pos, Cell { value, formula: formula.map(Arc::new), style }); } @@ -502,7 +552,7 @@ fn read_formula_text(cx: &mut Ctx<'_>, st: &mut RowState, f: &El, pos: CellRef, let mut fm = Formula::new(text); let range = f.attr("ref").and_then(RangeRef::parse).unwrap_or(RangeRef::cell(pos)); if !range.is_single() { - st.dynamic.push(range); + push_dynamic(st, range); } if !dynamic { fm.array = Some(range); @@ -672,6 +722,12 @@ fn read_cf(cx: &mut Ctx<'_>, e: &El, sheet: &mut Sheet) { if let Some(id) = ext_id { cx.cf_ext_ids.push((id, sheet.cond_formats.len())); } + let rules = sheet.cond_formats.len(); + let area = ranges.iter().map(RangeRef::count).sum::().saturating_mul(1 + rules as u64); + if area > MAX_COND_CELLS { + cx.warn(format!("conditional formats past {MAX_COND_CELLS} kept range cells were dropped")); + break; + } sheet.cond_formats.push(CondFormat { ranges: ranges.clone(), rule, @@ -734,6 +790,7 @@ fn read_validation(v: &El) -> Option { fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel], sheet: &mut Sheet) { let mut cells = 0u64; + let mut bytes = 0u64; for h in e.kids("hyperlink") { let Some(range) = h.attr("ref").and_then(RangeRef::parse) else { continue }; let mut target = h.attr("id").and_then(|id| rels.iter().find(|r| r.id == id)).map(|r| r.target.clone()).unwrap_or_default(); @@ -749,13 +806,19 @@ fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel], sheet: &m if target.is_empty() { continue; } - let link = Hyperlink { target, tooltip: h.attr("tooltip").map(str::to_string), external }; + let link = Hyperlink { target: target.clone(), tooltip: h.attr("tooltip").map(str::to_string), external }; + let link_bytes = (target.len() + link.tooltip.as_ref().map_or(0, |t| t.len())) as u64; for c in range.iter() { if cells >= MAX_HYPERLINK_CELLS { cx.warn(format!("sheet \"{name}\": hyperlinks past the first {MAX_HYPERLINK_CELLS} cells were dropped")); return; } + if bytes.saturating_add(link_bytes) > MAX_HYPERLINK_BYTES { + cx.warn(format!("sheet \"{name}\": hyperlink targets past {MAX_HYPERLINK_BYTES} bytes were dropped")); + return; + } cells += 1; + bytes += link_bytes; sheet.hyperlinks.insert(c, link.clone()); } } diff --git a/crates/xlsx/src/testing.rs b/crates/xlsx/src/testing.rs index 348c08af5..5729250bb 100644 --- a/crates/xlsx/src/testing.rs +++ b/crates/xlsx/src/testing.rs @@ -9,8 +9,8 @@ use aes::cipher::{BlockCipherEncrypt, KeyInit}; use crate::crypto::{ - BLOCK_KEY, BLOCK_VERIFIER_HASH, BLOCK_VERIFIER_INPUT, F_AES, F_CRYPTOAPI, Hash, PASSWORD_ENCRYPTOR, Params, SEGMENT, derive, iterated_hash, - standard_key, + BLOCK_HMAC_KEY, BLOCK_HMAC_VALUE, BLOCK_KEY, BLOCK_VERIFIER_HASH, BLOCK_VERIFIER_INPUT, F_AES, F_CRYPTOAPI, Hash, PASSWORD_ENCRYPTOR, Params, + SEGMENT, derive, hmac, iterated_hash, standard_key, }; /// The spin count `encrypt_workbook` uses: Excel's default. @@ -21,6 +21,7 @@ const KEY_DATA_SALT: [u8; 16] = [16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, const SECRET_KEY: [u8; 32] = [64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95]; const VERIFIER: [u8; 16] = [32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47]; +const HMAC_SALT: [u8; 16] = [112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127]; /// `package` (normally an `.xlsx` zip) as Excel stores it with a password: Agile encryption /// (AES-256, SHA-512) inside a compound file. @@ -202,8 +203,9 @@ fn package(size: usize, body: Vec) -> Vec { p } -/// The Agile XML descriptor: AES-256, SHA-512, a password encryptor salted with 0..16. -pub(crate) fn agile_info(password: &str, spin: u32) -> String { +/// The Agile XML descriptor: AES-256, SHA-512, a password encryptor salted with 0..16, and the +/// dataIntegrity pair (`hmac_key_b64`/`hmac_value_b64`) a compliant file carries. +pub(crate) fn agile_info(password: &str, spin: u32, hmac_key_b64: &str, hmac_value_b64: &str) -> String { let key = Params { hash: Hash::Sha512, salt: PASSWORD_SALT.to_vec(), key_len: 32 }; let h = iterated_hash(Hash::Sha512, password, &key.salt, spin); let iv = key.iv(None); @@ -217,9 +219,10 @@ pub(crate) fn agile_info(password: &str, spin: u32) -> String { base64_encode(salt) ) }; + // The dataIntegrity pair ([MS-OFFCRYPTO] 2.3.4.14), encrypted with the package key. format!( - "\r\n", - params(&KEY_DATA_SALT), + "\r\n", + params(KEY_DATA_SALT.as_slice()), params(&PASSWORD_SALT), ) } @@ -231,14 +234,20 @@ pub(crate) fn with_agile_header(xml: &str) -> Vec { info } -/// `EncryptionInfo` and `EncryptedPackage` for `plain`, Agile (AES-256, SHA-512). +/// `EncryptionInfo` and `EncryptedPackage` for `plain`, Agile (AES-256, SHA-512), with the +/// dataIntegrity HMAC over the whole EncryptedPackage stream as Excel writes it. pub(crate) fn encrypt_agile(plain: &[u8], password: &str, spin: u32) -> (Vec, Vec) { let key_data = Params { hash: Hash::Sha512, salt: KEY_DATA_SALT.to_vec(), key_len: 32 }; let mut body = vec![]; for (i, segment) in plain.chunks(SEGMENT).enumerate() { body.extend(encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&(i as u32).to_le_bytes())), segment)); } - (with_agile_header(&agile_info(password, spin)), package(plain.len(), body)) + let stream = package(plain.len(), body); + let (hmac_key_b64, hmac_value_b64) = ( + base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_KEY)), &HMAC_SALT)), + base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_VALUE)), &hmac(Hash::Sha512, &HMAC_SALT, &stream))), + ); + (with_agile_header(&agile_info(password, spin, &hmac_key_b64, &hmac_value_b64)), stream) } /// `EncryptionInfo` and `EncryptedPackage` for `plain`, Standard (AES-128, SHA-1). diff --git a/crates/xlsx/src/tests/encrypted_tests.rs b/crates/xlsx/src/tests/encrypted_tests.rs index 7a1a9bb82..525ed9193 100644 --- a/crates/xlsx/src/tests/encrypted_tests.rs +++ b/crates/xlsx/src/tests/encrypted_tests.rs @@ -56,7 +56,8 @@ fn schemes_no_password_opens_are_named_before_asking_for_one() { let file = |info: &[u8]| compound_file(&[("EncryptionInfo", info), ("EncryptedPackage", &[0; 8])]); assert_eq!(check_encryption(&file(&[3, 0, 2, 0, 4, 0, 0, 0])), unsupported("RC4")); assert_eq!(check_encryption(&file(&[4, 0, 3, 0, 0, 0, 0, 0])), unsupported("Extensible")); - let cert = agile_info("pw", 1000).replace("keyEncryptor/password", "keyEncryptor/certificate"); + let cert = + agile_info("pw", 1000, "AAAAAAAAAAAAAAAAAAAAAA==", "AAAAAAAAAAAAAAAAAAAAAA==").replace("keyEncryptor/password", "keyEncryptor/certificate"); assert_eq!(check_encryption(&file(&with_agile_header(&cert))), unsupported("certificate")); assert!(matches!(check_encryption(&file(&[4, 0])), Err(DecryptError::Malformed(_)))); // Rights management (IRM) keeps an EncryptedPackage but no EncryptionInfo. diff --git a/crates/xlsx/src/xls/mod.rs b/crates/xlsx/src/xls/mod.rs index ceaf3e26d..ec5c056d9 100644 --- a/crates/xlsx/src/xls/mod.rs +++ b/crates/xlsx/src/xls/mod.rs @@ -13,7 +13,7 @@ mod formula; mod ftab; mod records; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::Arc; use gridcraft_core::{CellRef, DateSystem, MAX_COLS, MAX_ROWS, RangeRef, Value}; @@ -59,6 +59,14 @@ pub(crate) fn is_xls(bytes: &[u8]) -> bool { cf.has_root_stream("Workbook") || cf.has_root_stream("Book") } +/// Most shared formulas one sheet expands, and the longest template text one attach may +/// produce: a template shared by its followers was costing every one of them a full decode and +/// print however often that could never succeed, so each anchor is tried once and failed ones +/// are skipped for the rest of the sheet. Beyond the cap the remaining references keep their +/// cached values. +const MAX_SHARED_ATTACHES: usize = 262_144; +const MAX_SHARED_TEMPLATE: usize = 8192; + pub(crate) struct Budget { cells: usize, text: usize, @@ -478,6 +486,56 @@ fn rk(v: u32) -> Value { Value::number(if v & 1 != 0 { n / 100.0 } else { n }) } +/// Attaches every pending formula to its anchor's template, returning how many shared +/// templates were expanded. An anchor whose template can't be imported is refused once: +/// its followers re-decode the same template for nothing, so they are skipped instead (they +/// keep their cached values). +fn resolve_pending( + st: &mut SheetState<'_>, + arrays: &HashMap, + shared: &HashMap, + pending: &[Pending], +) -> usize { + let mut failed: HashSet = HashSet::new(); + let mut shared_attaches = 0usize; + for p in pending { + if let Some((range, f)) = arrays.get(&p.anchor) { + if p.pos == p.anchor { + st.attach(p.pos, f, Mode::Cell, Some(*range)); + } + } else if let Some(f) = shared.get(&p.anchor) { + if failed.contains(&p.anchor) || shared_attaches >= MAX_SHARED_ATTACHES { + continue; + } + shared_attaches += 1; + st.attach(p.pos, f, Mode::Shared, None); + let imported = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).is_some_and(|f| f.text.chars().count() <= MAX_SHARED_TEMPLATE); + if !imported { + failed.insert(p.anchor); + } + } else { + st.unsupported("a shared formula that is missing from the file"); + } + } + if shared_attaches >= MAX_SHARED_ATTACHES { + st.unsupported("more shared formulas than the expansion cap; some formulas kept their values"); + } + shared_attaches +} + +/// The other cells of a legacy array formula hold only cached results: dropped, as when an +/// .xlsx is read, so that the formula fills its range on recalculation. If the formula itself +/// couldn't be imported, they keep their values. +fn blank_array_followers(st: &mut SheetState<'_>, pending: &[Pending]) { + for p in pending { + let filled = st.sheet.cell(p.anchor).and_then(|c| c.formula.as_ref()).and_then(|f| f.array).is_some_and(|r| r.contains(p.pos)); + if p.pos != p.anchor && filled { + let style = st.sheet.style_id(p.pos); + st.sheet.cells.set(p.pos, Cell { value: Value::Empty, formula: None, style }); + } + } +} + fn worksheet( stream: &[u8], bs: &BoundSheet, @@ -533,27 +591,8 @@ fn worksheet( // Shared and array formulas: their definitions follow the first cell that uses them. let (arrays, shared) = (std::mem::take(&mut st.arrays), std::mem::take(&mut st.shared)); let pending = std::mem::take(&mut st.pending); - for p in &pending { - if let Some((range, f)) = arrays.get(&p.anchor) { - if p.pos == p.anchor { - st.attach(p.pos, f, Mode::Cell, Some(*range)); - } - } else if let Some(f) = shared.get(&p.anchor) { - st.attach(p.pos, f, Mode::Shared, None); - } else { - st.unsupported("a shared formula that is missing from the file"); - } - } - // The other cells of a legacy array formula hold only cached results: dropped, as when an - // .xlsx is read, so that the formula fills its range on recalculation. If the formula itself - // couldn't be imported, they keep their values. - for p in &pending { - let filled = st.sheet.cell(p.anchor).and_then(|c| c.formula.as_ref()).and_then(|f| f.array).is_some_and(|r| r.contains(p.pos)); - if p.pos != p.anchor && filled { - let style = st.sheet.style_id(p.pos); - st.sheet.cells.set(p.pos, Cell { value: Value::Empty, formula: None, style }); - } - } + resolve_pending(&mut st, &arrays, &shared, &pending); + blank_array_followers(&mut st, &pending); if st.dropped > 0 { report.warn(format!("{} cells outside the grid were skipped.", st.dropped)); } diff --git a/crates/xlsx/src/xls/records.rs b/crates/xlsx/src/xls/records.rs index fac7847dd..047170d58 100644 --- a/crates/xlsx/src/xls/records.rs +++ b/crates/xlsx/src/xls/records.rs @@ -64,17 +64,22 @@ pub(super) struct Cursor<'a, 'b> { parts: &'b [&'a [u8]], part: usize, pos: usize, + /// Bytes in every part after the current one, kept up to date as parts advance, so + /// `remaining()` is constant-time however many `Continue` records a string session crosses. + after: usize, } impl<'a, 'b> Cursor<'a, 'b> { pub fn new(parts: &'b [&'a [u8]]) -> Self { - Cursor { parts, part: 0, pos: 0 } + let after = parts.iter().skip(1).map(|p| p.len()).sum::(); + Cursor { parts, part: 0, pos: 0, after } } /// Skips to the next part when the current one is used up. Returns whether a new part began. fn settle(&mut self) -> bool { let mut moved = false; while self.parts.get(self.part).is_some_and(|p| self.pos >= p.len()) && self.part + 1 < self.parts.len() { + self.after = self.after.saturating_sub(self.parts[self.part + 1].len()); self.part += 1; self.pos = 0; moved = true; @@ -112,10 +117,10 @@ impl<'a, 'b> Cursor<'a, 'b> { Ok(()) } - /// Bytes left in the record (all parts). + /// Bytes left in the record (all parts), constant-time in the part count. pub fn remaining(&self) -> usize { let here = self.parts.get(self.part).map_or(0, |p| p.len().saturating_sub(self.pos)); - here + self.parts.iter().skip(self.part + 1).map(|p| p.len()).sum::() + here.saturating_add(self.after) } /// The rest of the record as one buffer. @@ -129,6 +134,7 @@ impl<'a, 'b> Cursor<'a, 'b> { } self.part = self.parts.len(); self.pos = 0; + self.after = 0; out } @@ -195,4 +201,23 @@ mod tests { let mut c = Cursor::new(&parts); assert!(c.chars(MAX_CHARS + 1, false).is_err()); } + + #[test] + fn remaining_counts_every_part_wherever_the_cursor_is() { + let parts: [&[u8]; 4] = [&[1, 2, 3], &[2, 3, 4, 5], &[3, 4, 5, 6, 7], &[4]]; + let mut c = Cursor::new(&parts); + assert_eq!(c.remaining(), 13); + for b in 1..=3u64 { + assert_eq!(u64::from(c.u8().unwrap()), b, "part 0 in order"); + } + assert_eq!(c.remaining(), 10, "after the first part is used up"); + for _ in 0..4 { + c.u8().unwrap(); + } + assert_eq!(c.remaining(), 6, "into the Continue parts"); + let mut c = Cursor::new(&parts); + assert_eq!(u64::from(c.u8().unwrap()), 1); + assert_eq!(c.rest().len(), 12); + assert_eq!(c.remaining(), 0); + } } diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index ec8cd9e75..e671d0adf 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -13,6 +13,12 @@ use crate::IoError; /// Maximum element nesting accepted in any part. pub const MAX_DEPTH: usize = 256; +/// Most elements kept in one part's parsed XML tree: an emptied `El` costs well over a hundred +/// bytes, so a highly-compressible part of endless empty elements could fit the package byte +/// limit and still take gigabytes of tree before any semantic check ran. Parts whose big runs +/// matter (worksheet rows, shared strings) are streamed instead and never hold this tree. +pub const MAX_XML_NODES: usize = 1_000_000; + /// An element: local name (namespace prefix dropped), attributes by local name, children and /// its direct text content. #[derive(Clone, Debug, Default, PartialEq)] @@ -223,13 +229,27 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res let mut stack: Vec = Vec::new(); let mut root: Option = None; let mut buf = Vec::new(); + // Elements kept in the tree so far, so a pathological part that passes the byte limits can + // still be refused before the tree itself outgrows memory. + let mut kept = 0usize; // Attach a finished element to its parent (or emit it / make it the root). - fn finish(stack: &mut [El], root: &mut Option, el: El, split: &[&str], cb: &mut dyn FnMut(El) -> Result<(), IoError>) -> Result<(), IoError> { + fn finish( + stack: &mut [El], + root: &mut Option, + el: El, + split: &[&str], + cb: &mut dyn FnMut(El) -> Result<(), IoError>, + kept: &mut usize, + ) -> Result<(), IoError> { match stack.last_mut() { Some(parent) => { if split.contains(&el.name.as_str()) { cb(el)?; } else { + *kept = kept.saturating_add(1); + if *kept > MAX_XML_NODES { + return Err(IoError::Xml(format!("the part holds more than {MAX_XML_NODES} elements"))); + } parent.children.push(el); } } @@ -252,11 +272,11 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res } Event::Empty(e) => { let el = start_el(&e); - finish(&mut stack, &mut root, el, split, cb)?; + finish(&mut stack, &mut root, el, split, cb, &mut kept)?; } Event::End(_) => { if let Some(el) = stack.pop() { - finish(&mut stack, &mut root, el, split, cb)?; + finish(&mut stack, &mut root, el, split, cb, &mut kept)?; } } Event::Text(t) => { @@ -295,7 +315,7 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res } // Unclosed elements (truncated part): close them so we keep what we have. while let Some(el) = stack.pop() { - finish(&mut stack, &mut root, el, split, cb)?; + finish(&mut stack, &mut root, el, split, cb, &mut kept)?; } root.ok_or_else(|| IoError::Xml("no root element".into())) } @@ -475,6 +495,14 @@ mod tests { } } + #[test] + fn a_part_of_endless_empty_elements_is_refused() { + assert!(parse(b"").is_ok()); + let xml = format!("{}", "".repeat(MAX_XML_NODES + 1)); + let err = parse(xml.as_bytes()).unwrap_err(); + assert!(err.to_string().contains("elements"), "{err}"); + } + #[test] fn phonetic_data_validates_offsets() { let x = r#"課きくカクx"#; diff --git a/packaging/macos/import-cert.sh b/packaging/macos/import-cert.sh index 4611ffe05..ce9432e4e 100755 --- a/packaging/macos/import-cert.sh +++ b/packaging/macos/import-cert.sh @@ -17,12 +17,16 @@ if [[ -z "${APPLE_CERTIFICATE:-}" ]]; then exit 0 fi -TMP="${RUNNER_TEMP:-${TMPDIR:-/tmp}}" -KEYCHAIN="$TMP/gridcraft-signing.keychain-db" -CERT="$TMP/gridcraft-signing.p12" +BASE="${RUNNER_TEMP:-${TMPDIR:-/tmp}}" +# A private directory created exclusively, not predictable names in a possibly shared temp +# root: another local user's symlink at `gridcraft-signing.p12` would otherwise select the +# write target and capture the decoded certificate. +WORK="$(mktemp -d "$BASE/gridcraft-signing.XXXXXXXX")" +KEYCHAIN="$WORK/gridcraft-signing.keychain-db" +CERT="$WORK/gridcraft-signing.p12" KC_PASS="${KEYCHAIN_PASSWORD:-$(openssl rand -hex 24)}" -trap 'rm -f "$CERT"' EXIT +trap 'rm -rf "$WORK"' EXIT printf '%s' "$APPLE_CERTIFICATE" | base64 --decode >"$CERT" security create-keychain -p "$KC_PASS" "$KEYCHAIN" security set-keychain-settings -lut 21600 "$KEYCHAIN" diff --git a/xtask/src/stats.rs b/xtask/src/stats.rs index f40b1300a..462f1c7de 100644 --- a/xtask/src/stats.rs +++ b/xtask/src/stats.rs @@ -2,6 +2,9 @@ use std::path::Path; +#[cfg(unix)] +use std::os::unix::fs::MetadataExt as _; + #[derive(Default, Clone, Copy)] struct Counts { files: usize, @@ -10,7 +13,24 @@ struct Counts { proptests: usize, } -fn walk(dir: &Path, c: &mut Counts) { +fn walk(dir: &Path, c: &mut Counts, seen: &mut std::collections::HashSet, budget: &mut usize) { + if *budget == 0 { + // Hard stop for pathological trees: two directory symlinks pointing at each other + // would otherwise be walked exponentially. + return; + } + // Directory identities (dev+inode on unix) make a symlink to a visited directory a + // no-op, not another exponential branch of the scan. + #[cfg(unix)] + let id = std::fs::metadata(dir).ok().and_then(|m| m.dev().checked_mul(1 << 60).and_then(|d| d.checked_add(m.ino()))); + #[cfg(not(unix))] + let id: Option = None; + if let Some(id) = id + && !seen.insert(id) + { + return; + } + *budget -= 1; let Ok(rd) = std::fs::read_dir(dir) else { return; }; @@ -18,10 +38,11 @@ fn walk(dir: &Path, c: &mut Counts) { let p = e.path(); let name = e.file_name(); let name = name.to_string_lossy(); + // A symlinked directory is descended only if its identity is new (checked above). if p.is_dir() { // Nested fuzz workspaces and build output are not part of the crate. if name != "target" && name != "fuzz" && !name.starts_with('.') { - walk(&p, c); + walk(&p, c, seen, budget); } } else if name.ends_with(".rs") && let Ok(s) = std::fs::read_to_string(&p) @@ -71,7 +92,7 @@ pub fn run(_root: &Path, exact: bool) -> Result<(), String> { continue; }; let mut c = Counts::default(); - walk(&dir, &mut c); + walk(&dir, &mut c, &mut std::collections::HashSet::new(), &mut 10_000); let ex = if exact { exact_tests(&name) } else { None }; rows.push((name, c, ex)); }