Merge pull request #2161 from Barzabel/fix/2124-2125-expr-nesting

expr: bound runtime parser and native recursion depth (#2124, #2125)

crates/expr/src/prelude.js

@@ -670,14 +670,17 @@ var __groupHandler = {
   },
 };
 
-function __mkGroup(c, l, info) {
-  var f = function (k) { return f.__get(k); };
-  delete f.name;
-  delete f.length;
-  Object.setPrototypeOf(f, __GroupProto);
-  f.__c = c; f.__l = l; f.__p = info[0]; f.__info = info;
-  return new Proxy(f, __groupHandler);
-}
+// `Proxy` is private to the object model (see "Stack safety" at the end).
+var __mkGroup = (function (P) {
+  return function (c, l, info) {
+    var f = function (k) { return f.__get(k); };
+    delete f.name;
+    delete f.length;
+    Object.setPrototypeOf(f, __GroupProto);
+    f.__c = c; f.__l = l; f.__p = info[0]; f.__info = info;
+    return new P(f, __groupHandler);
+  };
+})(Proxy);
 
 // Properties ------------------------------------------------------------------------------------
 
@@ -852,9 +855,11 @@ var __propHandler = {
   },
 };
 
-function __mkProp(c, l, path) {
-  return new Proxy(new Prop(c, l, path), __propHandler);
-}
+var __mkProp = (function (P) {
+  return function (c, l, path) {
+    return new P(new Prop(c, l, path), __propHandler);
+  };
+})(Proxy);
 
 // Text styles (the Source Text style API) ------------------------------------------------------
 // A TextStyle is the source document (as JSON from the host), the character index it reads its
@@ -1069,3 +1074,101 @@ function velocityAtTime(t) { return thisProperty.velocityAtTime(t); }
 function speedAtTime(t) { return thisProperty.speedAtTime(t); }
 function key(n) { return thisProperty.key(n); }
 function nearestKey(t) { return thisProperty.nearestKey(t); }
+
+// ---------------------------------------------------------------------------------------------
+// Stack safety (#2124, #2125). A stack overflow aborts the whole app, so nothing a script does
+// may nest native code without bound. The expression text is checked for nesting before it is
+// compiled; source handed to `eval` and the function constructors at run time is checked by
+// `__nest`. The natives that walk nested values recursively (`join`, which `toString` and string
+// conversion use, `toLocaleString`, `JSON.stringify`, `flat`) call `__stack` once per level,
+// 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 define = Object.defineProperty, describe = Object.getOwnPropertyDescriptor, proto = Object.getPrototypeOf;
+  // Replace `obj[name]` keeping its attributes.
+  function replace(obj, name, f) {
+    var d = describe(obj, name);
+    define(obj, name, { value: f, writable: d.writable, enumerable: d.enumerable, configurable: true });
+  }
+  function guard(obj, name) {
+    var f = obj[name];
+    replace(obj, name, function () { stack(); return apply(f, this, arguments); });
+  }
+
+  // Indirect on purpose: a wrapper can't make a direct eval, so `eval` sees globals and `var`s
+  // but not `let`/`const` locals of the expression. Keeping direct eval would mean leaving it
+  // unchecked; expressions practically never need their locals inside eval.
+  var evalJs = eval;
+  eval = function (x) {
+    if (typeof x === 'string') {
+      stack();
+      nest(x);
+    }
+    return evalJs(x);
+  };
+  // `Function` and the generator / async variants (reachable as `(function* () {}).constructor`).
+  function dynamic(F) {
+    var w = function () {
+      var args = [];
+      for (var i = 0; i < arguments.length; i++) args.push(String(arguments[i]));
+      stack();
+      nest(args.slice(0, -1).join(','));
+      nest(args.length ? args[args.length - 1] : '');
+      return construct(F, args);
+    };
+    define(w, 'name', { value: F.name, configurable: true });
+    w.prototype = F.prototype;
+    replace(F.prototype, 'constructor', w);
+    return w;
+  }
+  Function = dynamic(Function);
+  dynamic(proto(function* () {}).constructor);
+  dynamic(proto(async function () {}).constructor);
+  dynamic(proto(async function* () {}).constructor);
+
+  guard(Array.prototype, 'join');
+  guard(Array.prototype, 'toLocaleString');
+  var flat = Array.prototype.flat;
+  replace(Array.prototype, 'flat', function (depth) {
+    stack();
+    var d = depth === undefined ? 1 : Number(depth);
+    return apply(flat, this, [d > 64 ? 64 : d]);
+  });
+
+  // A replacer function sees every value before it is serialized: the per-level check. An array
+  // replacer (a list of keys) becomes one that copies each object's listed keys, in list order.
+  // Each source object gets one copy per call, so a cycle in the source is a cycle of copies and
+  // the native check reports it (TypeError); an object shared without a cycle is still fine.
+  var stringify = JSON.stringify, MapJs = Map;
+  replace(JSON, 'stringify', function (value, replacer, space) {
+    var fn = typeof replacer === 'function' ? replacer : null;
+    var keys = null, copies = new MapJs();
+    if (Array.isArray(replacer)) {
+      keys = [];
+      for (var i = 0; i < replacer.length; i++) {
+        var k = replacer[i];
+        if (typeof k === 'number' || k instanceof Number || k instanceof String) k = String(k);
+        if (typeof k === 'string' && keys.indexOf(k) < 0) keys.push(k);
+      }
+    }
+    return apply(stringify, JSON, [value, function (key, v) {
+      stack();
+      if (fn !== null) v = apply(fn, this, [key, v]);
+      if (keys !== null && v !== null && typeof v === 'object' && !Array.isArray(v) &&
+          !(v instanceof Number || v instanceof String || v instanceof Boolean)) {
+        var o = copies.get(v);
+        if (o === undefined) {
+          o = {};
+          for (var j = 0; j < keys.length; j++) if (keys[j] in v) o[keys[j]] = v[keys[j]];
+          copies.set(v, o);
+        }
+        v = o;
+      }
+      return v;
+    }, space]);
+  });
+
+  delete globalThis.Proxy;
+  delete globalThis.Reflect;
+})();

crates/expr/src/rewrite.rs

@@ -805,6 +805,12 @@ pub fn check_nesting(src: &str) -> Result<(), String> {
     Ok(())
 }
 
+/// An error when plain JavaScript `src` (not rewritten: what `eval` and `Function` get at run
+/// time) nests deeper than [`MAX_NESTING`] levels.
+pub fn check_plain_nesting(src: &str) -> Result<(), String> {
+    if too_nested(&tokenize(src).0) { Err(too_deep()) } else { Ok(()) }
+}
+
 fn too_deep() -> String {
     format!("Error: expression is nested too deeply (more than {MAX_NESTING} levels of brackets, or too long a chain of operators)")
 }

crates/expr/src/runtime.rs

@@ -51,6 +51,8 @@ thread_local! {
     pub static FRAMES: RefCell<Vec<Frame>> = const { RefCell::new(Vec::new()) };
     /// Script runs since the outermost evaluation on this thread began (nested ones included).
     pub static RUNS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
+    /// Stack address where the current script run began (0: none; see [`native_stack`]).
+    static STACK_BASE: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
 }
 
 #[cfg(not(target_arch = "wasm32"))]
@@ -232,17 +234,54 @@ fn native_random(_: &JsValue, args: &[JsValue], ctx: &mut Context) -> JsResult<J
     Ok(JsValue::from(noise::unit(noise::mix(&parts))))
 }
 
+/// Stack a script run may use below [`run`] for natives that recurse once per level of a nested
+/// value (`join`, `JSON.stringify`, …; the prelude calls [`native_stack`] per level). It is the
+/// same on every thread, so a value converts or fails alike in preview, export and commands.
+/// Expressions run on render, export and diagnostics threads (2 MiB, std and rayon's default)
+/// and on the main thread for commands and syntax checks (1 MiB on Windows). A debug build
+/// stops the nested-value tests within a fresh 512 KiB thread, which leaves the main thread
+/// 512 KiB for its callers. The parser's limit ([`crate::rewrite::MAX_NESTING`]) is the tighter
+/// one there: 30 levels need about 1 MiB in debug builds. On wasm32 the address of a local is on
+/// the shadow stack, not the engine's call stack, so this measures nothing useful there.
+const NATIVE_STACK: usize = 256 << 10;
+
+/// An address on the current stack (on wasm32, the shadow stack).
+fn stack_here() -> usize {
+    let marker = 0u8;
+    std::hint::black_box(&marker) as *const u8 as usize
+}
+
+/// `__stack()`: throw once the current run has used [`NATIVE_STACK`] (values nested so deeply
+/// would overflow the stack, which aborts the app).
+fn native_stack(_: &JsValue, _: &[JsValue], _: &mut Context) -> JsResult<JsValue> {
+    let base = STACK_BASE.with(|b| b.get());
+    if base != 0 && base.abs_diff(stack_here()) > NATIVE_STACK {
+        return Err(JsNativeError::range().with_message("value is nested too deeply").into());
+    }
+    Ok(JsValue::undefined())
+}
+
+/// `__nest(src)`: throw when source handed to `eval` / `Function` at run time nests deeper than
+/// the parser's stack allows (the expression text itself is checked before it is compiled).
+fn native_nesting(_: &JsValue, args: &[JsValue], ctx: &mut Context) -> JsResult<JsValue> {
+    let src = string(args, 0, ctx)?;
+    crate::rewrite::check_plain_nesting(&src).map_err(|e| JsNativeError::range().with_message(e.trim_start_matches("Error: ").to_string()))?;
+    Ok(JsValue::undefined())
+}
+
 impl Runtime {
     fn new() -> Result<Runtime, String> {
         let mut ctx = Context::default();
         let limits = ctx.runtime_limits_mut();
         limits.set_loop_iteration_limit(LOOP_LIMIT);
         limits.set_recursion_limit(400);
-        let natives: [(JsString, usize, fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue>); 4] = [
+        let natives: [(JsString, usize, fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue>); 6] = [
+            (js_string!("__stack"), 0, native_stack),
             (js_string!("__h"), 6, native_host),
             (js_string!("__wig"), 8, native_wiggle),
             (js_string!("__noise"), 3, native_noise),
             (js_string!("__rnd"), 6, native_random),
+            (js_string!("__nest"), 1, native_nesting),
         ];
         for (name, len, f) in natives {
             ctx.register_global_callable(name, len, NativeFunction::from_fn_ptr(f)).map_err(|e| e.to_string())?;
@@ -361,6 +400,9 @@ pub fn out_of_time() -> bool {
     }
 }
 
+/// Levels of nested arrays a script result keeps.
+const MAX_DETACH: usize = 4;
+
 /// Copy a JS value out of the heap.
 fn detach(v: &JsValue, ctx: &mut Context, depth: usize) -> Result<Out, String> {
     if v.is_undefined() {
@@ -380,7 +422,7 @@ fn detach(v: &JsValue, ctx: &mut Context, depth: usize) -> Result<Out, String> {
     }
     let err = |e: boa_engine::JsError| format_error(&e.to_string(), usize::MAX);
     if let Some(o) = v.as_object() {
-        if o.is_array() && depth < 4 {
+        if o.is_array() && depth < MAX_DETACH {
             let arr = JsArray::from_object(o).map_err(err)?;
             let n = arr.length(ctx).map_err(err)?.min(1 << 16);
             let mut out = Vec::with_capacity(n as usize);
@@ -408,6 +450,11 @@ fn detach(v: &JsValue, ctx: &mut Context, depth: usize) -> Result<Out, String> {
             return Ok(Out::Path { points, ins, outs, closed });
         }
     }
+    // Below the levels detached above, an object's text could be its whole nested content
+    // (`String([[[…]]])`), converted by natives that recurse once per level.
+    if depth >= MAX_DETACH && v.is_object() {
+        return Ok(Out::Object("[object]".into()));
+    }
     Ok(Out::Object(v.to_string(ctx).map(|s| s.to_std_string_escaped()).unwrap_or_default()))
 }
 
@@ -484,6 +531,8 @@ fn position_line(msg: &str) -> Option<usize> {
 /// `__begin` with memoized answers, so nothing carries over. A script that really overflows the
 /// stack fails again on the fresh context and reports the error.
 pub fn run(text: &str, b: &Begin) -> Run {
+    // Runs don't nest (other expressions are evaluated between runs), so one base suffices.
+    STACK_BASE.with(|s| s.set(stack_here()));
     RUNTIME.with(|r| {
         let mut r = r.borrow_mut();
         for fresh in [false, true] {

crates/expr/src/tests.rs

@@ -1,5 +1,6 @@
 use std::time::Instant;
 
+mod deep_nesting;
 mod fuzz;
 
 use effectcraft_color::Label;

crates/expr/src/tests/deep_nesting.rs

@@ -0,0 +1,130 @@
+//! Deep nesting the expression text check can't see (#2124, #2125): source handed to `eval` /
+//! `Function` at run time, and deeply nested values that boa's natives (`join`, `JSON.stringify`,
+//! proxies…) walk recursively. Each used to overflow the stack, which aborts the whole app
+//! (`catch_unwind` can't catch it); now each is an expression error.
+
+use super::*;
+
+/// The render threads' stack size (rayon's default): what the app gets.
+const RENDER_STACK: usize = 2 << 20;
+
+/// Evaluate `text` on the title's source text (any result converts) on a render-sized stack.
+fn eval_small_stack(text: &str) -> Result<Value, String> {
+    eval_on_stack(text, RENDER_STACK)
+}
+
+/// Evaluate `text` on the title's source text on a thread with `size` bytes of stack.
+fn eval_on_stack(text: &str, size: usize) -> Result<Value, String> {
+    let text = text.to_string();
+    std::thread::Builder::new()
+        .stack_size(size)
+        .spawn(move || {
+            let f = fx();
+            f.ev(f.title, "text/sourceText", &text, 0.0)
+        })
+        .unwrap()
+        .join()
+        .unwrap()
+}
+
+fn text_of(r: Result<Value, String>) -> String {
+    match r {
+        Ok(Value::Text(d)) => d.text,
+        other => panic!("not text: {other:?}"),
+    }
+}
+
+/// A 100 000-deep array: `var a = []; … a = [a]` in a loop (under the loop limit).
+const DEEP_ARRAY: &str = "var a = []; for (var i = 0; i < 100000; i++) a = [a];";
+
+#[test]
+fn eval_and_function_constructors_check_nesting() {
+    let deep = r#""(".repeat(1000) + "1" + ")".repeat(1000)"#;
+    for call in [
+        format!("eval({deep})"),
+        format!("Function({deep})()"),
+        format!("new Function('a', {deep})(1)"),
+        format!("Function('a = ' + {deep}, 'return a')()"),
+        "Function.apply(null, '('.repeat(1000).split('').concat(['1']))".to_string(),
+        format!("(function () {{}}).constructor({deep})()"),
+        format!("Object.getPrototypeOf(function* () {{}}).constructor({deep})"),
+        format!("Object.getPrototypeOf(async function () {{}}).constructor({deep})"),
+        format!("Object.getPrototypeOf(async function* () {{}}).constructor({deep})"),
+    ] {
+        let e = eval_small_stack(&call).expect_err(&call);
+        assert!(e.contains("nested too deeply"), "{call}: {e}");
+    }
+}
+
+#[test]
+fn eval_and_function_still_work() {
+    let f = fx();
+    assert_eq!(f.num("eval('1 + 2')", 0.0), 3.0);
+    assert_eq!(f.num("eval('[1, [2, (3)]]')[1][1]", 0.0), 3.0);
+    assert_eq!(f.num("new Function('a', 'b', 'return a * b')(3, 4)", 0.0), 12.0);
+    assert_eq!(f.num("Function('return 5')()", 0.0), 5.0);
+    assert_eq!(f.num("(function () {}).constructor('return 6')()", 0.0), 6.0);
+    assert_eq!(f.num("(function () {}) instanceof Function ? 1 : 0", 0.0), 1.0);
+    assert_eq!(f.num("(function () {}).constructor === Function ? 1 : 0", 0.0), 1.0);
+    assert_eq!(f.num("Object.getPrototypeOf(function* () { yield 7; }).constructor('yield 7')().next().value", 0.0), 7.0);
+}
+
+#[test]
+fn deep_array_result_is_an_error_or_text_not_a_crash() {
+    // The result itself: detaching it used to `toString` it past depth 4.
+    let _ = eval_small_stack(&format!("{DEEP_ARRAY} a"));
+    // Explicit conversions inside the script.
+    for tail in ["a.join()", "a.toString()", "String(a)", "a + ''", "`${a}`", "a.toLocaleString()", "JSON.stringify(a)", "JSON.stringify({ a: a })"] {
+        let text = format!("{DEEP_ARRAY} {tail}");
+        let e = eval_small_stack(&text).expect_err(&text);
+        assert!(e.contains("nested too deeply"), "{text}: {e}");
+    }
+    // A deep array as a text style value goes through JSON.stringify in the prelude.
+    let _ = eval_small_stack(&format!("{DEEP_ARRAY} text.sourceText.style.setFontSize(a)"));
+}
+
+#[test]
+fn deep_proxy_chain_is_an_error_not_a_crash() {
+    let text = "var p = {}; for (var i = 0; i < 100000; i++) p = new Proxy(p, {}); p.x";
+    let e = eval_small_stack(text).expect_err(text);
+    assert!(e.contains("Proxy") && e.contains("not defined"), "{text}: {e}");
+    // Neither is available to expressions; the object model's own proxies still work.
+    let f = fx();
+    assert_eq!(f.text("typeof Proxy + ' ' + typeof Reflect", 0.0), "undefined undefined");
+    assert_eq!(f.num("thisComp.layer('A').effect('Slider Control')('Slider')", 0.0), 42.0);
+}
+
+#[test]
+fn nested_values_still_convert() {
+    let f = fx();
+    assert_eq!(f.text("[[1, 2], [3, [4, 5]]].join(';')", 0.0), "1,2;3,4,5");
+    assert_eq!(f.text("String([[1, 2], 3])", 0.0), "1,2,3");
+    assert_eq!(f.text("JSON.stringify({ a: [1, { b: [2] }] })", 0.0), r#"{"a":[1,{"b":[2]}]}"#);
+    assert_eq!(f.text("JSON.stringify({ a: 1, b: 2 }, ['b'])", 0.0), r#"{"b":2}"#);
+    assert_eq!(f.text("JSON.stringify([1, [2]], null, 1)", 0.0), "[\n 1,\n [\n  2\n ]\n]");
+    assert_eq!(f.text("JSON.stringify({ a: 1 }, function (k, v) { return k === 'a' ? v + 1 : v; })", 0.0), r#"{"a":2}"#);
+    // Nested deeper than the detached levels: still converts to its text.
+    assert_eq!(text_of(eval_small_stack("[[[[[[1, 2]]]]]]")), "");
+    assert_eq!(f.text("String([[[[[[[[1]]]]]]]])", 0.0), "1");
+    assert_eq!(f.num("[1, [2, [3, [4]]]].flat(Infinity).length", 0.0), 4.0);
+}
+
+/// The main thread runs commands and syntax checks: 1 MiB on Windows. Half of it, for the
+/// callers above the evaluation, must still turn the nested-value triggers into errors.
+#[test]
+fn nested_values_are_an_error_on_half_the_main_thread() {
+    for tail in ["a.join()", "String([a])", "JSON.stringify(a)", "a.toLocaleString()"] {
+        let text = format!("{DEEP_ARRAY} {tail}");
+        let e = eval_on_stack(&text, 512 << 10).expect_err(&text);
+        assert!(e.contains("nested too deeply"), "{tail}: {e}");
+    }
+}
+
+#[test]
+fn stringify_with_a_key_list_reports_cycles_and_allows_shared_objects() {
+    let f = fx();
+    let cyclic = f.text("var o = {a: 1}; o.self = o; try { JSON.stringify(o, ['a', 'self']) } catch (e) { e.name }", 0.0);
+    assert_eq!(cyclic, "TypeError");
+    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}}"#);
+}