#786: fuzz expressions; fix the overflows, hangs and panic it found

Problem: expression text is input we don't control, and nothing fuzzed the
expression parser and evaluator. A seeded mutation fuzzer found stack
overflows (a few hundred `?:`/`=>`/`if`, a few thousand `.`/`<`/`()` links,
75 nested template substitutions or regex groups, a chain inside one
template), hangs (an expression reading its own layer's transform nested
2^16 times; fanning-out reads; 10M-iteration loops re-run per read; loops
calling looping functions), an i64 overflow panic in wiggle noise, and NaN
path vertices tripping the rasterizer's clipping assertion.

Fix: a dependency-free seeded fuzzer (crates/expr/src/tests/fuzz.rs: real
AE-style seeds, char/token mutations, worker thread with a 2 MiB stack,
catch_unwind, deadline, env vars for long runs); a weighted nesting
estimate covering chains without brackets and template/regex insides;
cycle detection per (property, time, text); a per-evaluation budget of
20 000 script runs and 5 s (runs are interrupted through boa's budgeted
evaluation; native only); loop limit 1M; wrapping noise lattice indices;
non-finite path results rejected. Each fix has a regression test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
vangel76
2026-10-11 14:40:03 +02:00
co-authored by Claude Opus 5.5
parent 9a7d3e7699
commit 9da66d9063
6 changed files with 820 additions and 28 deletions
+72 -2
View File
@@ -16,6 +16,11 @@ const PRELUDE: &str = include_str!("prelude.js");
/// Message of the error thrown when a request isn't answered yet (never shown to users).
const PENDING: &str = "\u{1}effectcraft: pending host request";
const MAX_SCRIPTS: usize = 4096;
/// Loop iterations one script run may make (per function call): a run re-executes once per
/// property it reads, so this bounds the time of every pass.
pub const LOOP_LIMIT: u64 = 1_000_000;
/// The error of an evaluation that ran out of time.
pub const TIMED_OUT: &str = "Error: expression takes too long to evaluate";
/// The part of boa's stack-size limit error the runtime recovers from (see [`run`]).
const STACK_LIMIT: &str = "maximum stack size";
@@ -24,6 +29,18 @@ const STACK_LIMIT: &str = "maximum stack size";
pub struct Frame {
pub memo: HashMap<Req, Resp>,
pub misses: Vec<Req>,
/// What this frame evaluates: an evaluation that needs itself is a cycle.
pub owner: Option<Owner>,
}
/// One expression evaluation: the property (comp, layer, uid), the comp time and the text.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Owner {
pub comp: u64,
pub layer: u64,
pub uid: u64,
pub time: effectcraft_time::Tick,
pub text: String,
}
thread_local! {
@@ -32,6 +49,14 @@ thread_local! {
static RUNTIME: RefCell<Option<ManuallyDrop<Result<Runtime, String>>>> = const { RefCell::new(None) };
/// Evaluation stack (nested when one expression reads another's value).
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) };
}
#[cfg(not(target_arch = "wasm32"))]
thread_local! {
/// When the outermost evaluation on this thread began.
pub static STARTED: std::cell::Cell<Option<std::time::Instant>> = const { std::cell::Cell::new(None) };
}
struct Runtime {
@@ -209,7 +234,7 @@ impl Runtime {
fn new() -> Result<Runtime, String> {
let mut ctx = Context::default();
let limits = ctx.runtime_limits_mut();
limits.set_loop_iteration_limit(10_000_000);
limits.set_loop_iteration_limit(LOOP_LIMIT);
limits.set_recursion_limit(400);
let natives: [(JsString, usize, fn(&JsValue, &[JsValue], &mut Context) -> JsResult<JsValue>); 4] = [
(js_string!("__h"), 6, native_host),
@@ -270,7 +295,11 @@ impl Runtime {
if let Err(e) = self.begin.call(&JsValue::undefined(), &args, ctx) {
return Run::Done(Err(format_error(&e.to_string(), line_count(text))));
}
let result = script.evaluate(ctx).and_then(|v| self.finish.call(&JsValue::undefined(), &[v], ctx));
let result = match evaluate_in_time(&script, ctx) {
Some(r) => r.and_then(|v| self.finish.call(&JsValue::undefined(), &[v], ctx)),
// Abandoned mid-run: the context is unusable now (see `run`).
None => return Run::Done(Err(TIMED_OUT.into())),
};
let pending = FRAMES.with(|f| f.borrow().last().is_some_and(|fr| !fr.misses.is_empty()));
if pending {
return Run::Pending;
@@ -282,6 +311,35 @@ impl Runtime {
}
}
/// Run `script`, giving up (`None`) once the outermost evaluation is out of time
/// ([`crate::MAX_TIME`]). boa's budgeted evaluation yields every few thousand instructions of
/// the script and the functions it calls (not inside callbacks of native functions such as
/// `Array.prototype.map`, which stay bounded by the loop limit per call).
fn evaluate_in_time(script: &Script, ctx: &mut Context) -> Option<JsResult<JsValue>> {
let mut run = std::pin::pin!(script.evaluate_async_with_budget(ctx, 20_000));
let mut cx = std::task::Context::from_waker(std::task::Waker::noop());
loop {
if let std::task::Poll::Ready(r) = run.as_mut().poll(&mut cx) {
return Some(r);
}
if out_of_time() {
return None;
}
}
}
/// Whether the outermost evaluation on this thread has used up [`crate::MAX_TIME`].
pub fn out_of_time() -> bool {
#[cfg(not(target_arch = "wasm32"))]
{
STARTED.with(|s| s.get()).is_some_and(|t| t.elapsed() > crate::MAX_TIME)
}
#[cfg(target_arch = "wasm32")]
{
false
}
}
/// Copy a JS value out of the heap.
fn detach(v: &JsValue, ctx: &mut Context, depth: usize) -> Result<Out, String> {
if v.is_undefined() {
@@ -413,6 +471,13 @@ pub fn run(text: &str, b: &Begin) -> Run {
Err(e) => return Run::Done(Err(e.clone())),
};
match out {
Run::Done(Err(e)) if e == TIMED_OUT => {
// Abandoned mid-run: start the next evaluation on a fresh context.
if let Some(old) = r.take() {
drop(ManuallyDrop::into_inner(old));
}
return Run::Done(Err(e));
}
Run::Done(Err(e)) if !fresh && e.contains(STACK_LIMIT) => {
// Dropped here, mid-thread, where boa's heap is alive (unlike at thread exit).
if let Some(old) = r.take() {
@@ -500,6 +565,11 @@ pub fn to_value(out: &Out, current: &Value) -> Result<Value, String> {
Value::Str(_) => Value::Str(text(out)),
Value::Path(_) => match out {
Out::Path { points, ins, outs, closed } => {
// A vertex that isn't a number (`[[0, 0], [[1, 2], 0]]`) comes through as NaN, which
// the rasterizer must never see.
if points.iter().chain(ins).chain(outs).flatten().any(|c| !c.is_finite()) {
return Err("Error: path points must be finite numbers".into());
}
let n = points.len();
let fit = |v: &[[f64; 2]]| {
let mut v = v.to_vec();