diff --git a/crates/expr/src/host.rs b/crates/expr/src/host.rs index fa2993f97..0738b3301 100644 --- a/crates/expr/src/host.rs +++ b/crates/expr/src/host.rs @@ -120,6 +120,9 @@ fn cell_json(c: &str) -> serde_json::Value { } } +/// Bytes of header names the row objects of delimited data may repeat in all. +const MAX_DATA_JSON_BYTES: usize = 64 << 20; + /// The data of a data footage item as `{data, rows, header}` JSON (see [`Req::FootageData`]). pub fn footage_data_json(f: &effectcraft_project::Footage) -> Option { let text = f.data.as_deref()?; @@ -135,6 +138,12 @@ pub fn footage_data_json(f: &effectcraft_project::Footage) -> Option { Some((h, b)) => (h.clone(), b.to_vec()), None => (vec![], vec![]), }; + // Every row object repeats every header name: a long header over many rows is far + // more than the text itself. + let keys: usize = header.iter().map(|k| k.len() + 4).sum(); + if keys.saturating_mul(body.len()) > MAX_DATA_JSON_BYTES { + return None; + } let objects: Vec = body .iter() .map(|r| { diff --git a/crates/expr/src/lib.rs b/crates/expr/src/lib.rs index 9e78d250f..758065c44 100644 --- a/crates/expr/src/lib.rs +++ b/crates/expr/src/lib.rs @@ -205,6 +205,12 @@ fn evaluate_out( Run::Pending => { let misses = FRAMES.with(|f| f.borrow_mut().get_mut(depth).map(|fr| std::mem::take(&mut fr.misses)).unwrap_or_default()); for m in misses { + // A request caught and repeated by the script is answered once; each answer + // can be costly (`sampleImage`), so the time budget is checked between them. + if FRAMES.with(|f| f.borrow().get(depth).is_some_and(|fr| fr.memo.contains_key(&m))) { + continue; + } + budget_left()?; // May evaluate other expressions (nested frames above ours). let r = resolver.resolve(&m); FRAMES.with(|f| f.borrow_mut().get_mut(depth).map(|fr| fr.memo.insert(m, r))); diff --git a/crates/expr/src/prelude.js b/crates/expr/src/prelude.js index c6fcbab03..70345b355 100644 --- a/crates/expr/src/prelude.js +++ b/crates/expr/src/prelude.js @@ -1187,7 +1187,7 @@ function nearestKey(t) { return thisProperty.nearestKey(t); } // which throws once the run has used its share of the stack. `Proxy` (chains of proxies nest // without any script frame) and `Reflect` are not available to expressions. (function () { - var nest = __nest, stack = __stack, apply = Reflect.apply, construct = Reflect.construct; + var nest = __nest, stack = __stack, apply = Reflect.apply, construct = Reflect.construct, str = String; var define = Object.defineProperty, describe = Object.getOwnPropertyDescriptor, proto = Object.getPrototypeOf; // Replace `obj[name]` keeping its attributes. function replace(obj, name, f) { @@ -1212,12 +1212,20 @@ function nearestKey(t) { return thisProperty.nearestKey(t); } }; // `Function` and the generator / async variants (reachable as `(function* () {}).constructor`). function dynamic(F) { + // Each argument is converted once, and the strings checked are the ones compiled: built + // in helpers (`push`, `slice`, `join`, `String`) can be replaced by the expression, and + // must not make the check see other text than the constructor gets. var w = function () { - var args = []; - for (var i = 0; i < arguments.length; i++) args.push(String(arguments[i])); + var n = arguments.length, args = [], params = ''; + for (var i = 0; i < n; i++) { + var s = str(arguments[i]); + if (typeof s !== 'string') s = ''; + define(args, i, { value: s, writable: true, enumerable: true, configurable: true }); + if (i < n - 1) params = i ? params + ',' + s : s; + } stack(); - nest(args.slice(0, -1).join(',')); - nest(args.length ? args[args.length - 1] : ''); + nest(params); + nest(n ? args[n - 1] : ''); return construct(F, args); }; define(w, 'name', { value: F.name, configurable: true }); diff --git a/crates/expr/src/runtime.rs b/crates/expr/src/runtime.rs index 84bfcdf3c..fe8db2bbc 100644 --- a/crates/expr/src/runtime.rs +++ b/crates/expr/src/runtime.rs @@ -346,7 +346,7 @@ impl Runtime { return Run::Pending; } Run::Done(match result { - Ok(v) => detach(&v, ctx, 0), + Ok(v) => detach(&v, ctx, 0, &mut MAX_DETACH_VALUES.clone()), Err(e) => Err(format_error(&e.to_string(), line_count(text))), }) } @@ -402,9 +402,18 @@ pub fn out_of_time() -> bool { /// Levels of nested arrays a script result keeps. const MAX_DETACH: usize = 4; +/// Values a script result may hold in all: one array can be referenced many times over, so +/// [`MAX_DETACH`] levels of 65 536 elements each would otherwise copy 2⁶⁴ of them. +const MAX_DETACH_VALUES: usize = 1 << 20; -/// Copy a JS value out of the heap. -fn detach(v: &JsValue, ctx: &mut Context, depth: usize) -> Result { +/// Copy a JS value out of the heap, counting each value against `left` (shared by the whole +/// result). Reading a result can run script (getters, conversions), so the time budget is +/// checked for each value too. +fn detach(v: &JsValue, ctx: &mut Context, depth: usize, left: &mut usize) -> Result { + *left = left.checked_sub(1).ok_or("Error: the expression's result holds too many values")?; + if out_of_time() { + return Err(TIMED_OUT.into()); + } if v.is_undefined() { return Ok(Out::Undefined); } @@ -427,25 +436,27 @@ fn detach(v: &JsValue, ctx: &mut Context, depth: usize) -> Result { let n = arr.length(ctx).map_err(err)?.min(1 << 16); let mut out = Vec::with_capacity(n as usize); for i in 0..n { - out.push(detach(&arr.get(i, ctx).map_err(err)?, ctx, depth + 1)?); + out.push(detach(&arr.get(i, ctx).map_err(err)?, ctx, depth + 1, left)?); } return Ok(Out::Arr(out)); } - if o.get(js_string!("__style"), ctx).map_err(err)?.to_boolean() { + // A style or path tag is only read within the levels detached: a tagged object whose + // fields lead back to itself would otherwise recurse until the stack overflows. + if depth < MAX_DETACH && o.get(js_string!("__style"), ctx).map_err(err)?.to_boolean() { let j = o.get(js_string!("__json"), ctx).map_err(err)?; return Ok(Out::Style(j.as_string().map(|s| s.to_std_string_escaped()).unwrap_or_default())); } - if o.get(js_string!("__path"), ctx).map_err(err)?.to_boolean() { - let pts = |name: JsString, ctx: &mut Context| -> Result, String> { + if depth < MAX_DETACH && o.get(js_string!("__path"), ctx).map_err(err)?.to_boolean() { + let pts = |name: JsString, ctx: &mut Context, left: &mut usize| -> Result, String> { let v = o.get(name, ctx).map_err(err)?; - Ok(match detach(&v, ctx, depth + 1)? { + Ok(match detach(&v, ctx, depth + 1, left)? { Out::Arr(items) => items.iter().map(|p| out_vec(p, 2)).map(|c| [c[0], c[1]]).collect(), _ => vec![], }) }; - let points = pts(js_string!("__pts"), ctx)?; - let ins = pts(js_string!("__in"), ctx)?; - let outs = pts(js_string!("__out"), ctx)?; + let points = pts(js_string!("__pts"), ctx, left)?; + let ins = pts(js_string!("__in"), ctx, left)?; + let outs = pts(js_string!("__out"), ctx, left)?; let closed = o.get(js_string!("__closed"), ctx).map_err(err)?.to_boolean(); return Ok(Out::Path { points, ins, outs, closed }); } diff --git a/crates/expr/src/tests.rs b/crates/expr/src/tests.rs index fd5de44fa..2f721adf7 100644 --- a/crates/expr/src/tests.rs +++ b/crates/expr/src/tests.rs @@ -1672,3 +1672,22 @@ fn an_expression_cant_change_the_prelude_for_the_next() { } mod property_group; + +/// Delimited data whose row objects would repeat a long header many times is refused. +#[test] +fn delimited_data_row_objects_are_bounded() { + let header = "h".repeat(1 << 20); + let mut text = format!("{header}\n"); + for _ in 0..100 { + text.push_str("1\n"); + } + let f = effectcraft_project::Footage { path: "/d.csv".into(), kind: effectcraft_project::FootageKind::Data, data: Some(text), ..Default::default() }; + assert!(crate::host::footage_data_json(&f).is_none()); + let f = effectcraft_project::Footage { + path: "/d.csv".into(), + kind: effectcraft_project::FootageKind::Data, + data: Some("a,b\n1,2\n".into()), + ..Default::default() + }; + assert!(crate::host::footage_data_json(&f).unwrap().contains("\"a\":1")); +} diff --git a/crates/expr/src/tests/deep_nesting.rs b/crates/expr/src/tests/deep_nesting.rs index ae78c5b20..292d23edf 100644 --- a/crates/expr/src/tests/deep_nesting.rs +++ b/crates/expr/src/tests/deep_nesting.rs @@ -128,3 +128,33 @@ fn stringify_with_a_key_list_reports_cycles_and_allows_shared_objects() { let shared = f.text("var x = {a: 1, b: 2}; JSON.stringify({p: x, q: x}, ['p', 'q', 'a'])", 0.0); assert_eq!(shared, r#"{"p":{"a":1},"q":{"a":1}}"#); } + +/// Array helpers replaced by the expression don't change what the `Function` guard checks: the +/// deeply nested parameter list it compiles is the one it checks. +#[test] +fn function_guard_checks_the_text_it_compiles() { + let params = format!("{}a{}", "[".repeat(20_000), "]".repeat(20_000)); + let text = format!( + "Array.prototype.join = function () {{ return ''; }}; Array.prototype.slice = function () {{ return []; }}; \ + Array.prototype.push = function () {{ return 0; }}; Function('{params}', 'return 1'); 'ran'" + ); + let e = eval_small_stack(&text).unwrap_err(); + assert!(e.contains("nested too deeply"), "{e}"); +} + +/// A path-tagged result whose points lead back to itself stops at the detach depth instead of +/// recursing until the stack overflows. +#[test] +fn a_cyclic_path_result_is_bounded() { + // Reaching the end (an error or a value) is the test: the overflow aborted the process. + let _ = eval_small_stack("var o = {__path: true}; o.__pts = o; o.__in = o; o.__out = o; o"); +} + +/// One array referenced at every level of a result is not copied once per reference past the +/// result's value budget. +#[test] +fn a_result_of_shared_arrays_is_bounded() { + let e = eval_small_stack("var a = new Array(65536).fill(0); var b = new Array(65536).fill(a); var c = new Array(65536).fill(b); c").unwrap_err(); + // Refused by the value budget, or by the deadline now checked at each value on a slow machine. + assert!(e.contains("too many values") || e.contains("too long"), "{e}"); +}