From b47a7104ac9abdf987619047a61f70ed77de8652 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 15:54:55 -0500 Subject: [PATCH] Scripting: run-time compiled code passes the nesting check; bounded file masks, socket line search and file reads (#2975) * Scripting: run-time compiled code passes the nesting check; bounded file masks, socket line search and file reads eval and the Function constructors compiled script-built strings without the bracket-nesting check the script itself passes, so a short script could overflow the parser's stack. A host hook now checks every dynamic compilation. Folder.getFiles turned masks into a backtracking regular expression; a two-pointer wildcard match replaces it. Socket readln rescanned the whole buffer after every read; only new bytes are searched. Script files that are not regular files (pipes, devices) are not read. * address review: check_nesting scans template interpolations, file masks keep wildcard length * address review: file masks fold case as /i does; the nesting check skips regular expression literals * address review: nesting scan reads ++/-- and keyword-named properties as division context * address review: property-name state set at each word start; a number's decimal point is not a property dot * address review: a leading hashbang line is a comment for the nesting scan; eval source is checked as it stands --- crates/script/src/prelude.js | 33 +++++- crates/script/src/runtime.rs | 196 +++++++++++++++++++++++++++++++- crates/script/src/scriptfile.rs | 4 + crates/script/src/socket.rs | 12 +- crates/script/src/tests.rs | 45 ++++++++ 5 files changed, 279 insertions(+), 11 deletions(-) diff --git a/crates/script/src/prelude.js b/crates/script/src/prelude.js index 37910bf9e..5adab6eec 100644 --- a/crates/script/src/prelude.js +++ b/crates/script/src/prelude.js @@ -292,6 +292,34 @@ function Folder(path) { if (!(this instanceof Folder)) return new Folder(path); this.__path = path === undefined ? "" : __str(path); } + +// ECMAScript's legacy case canonicalisation (what a `/i` regular expression compares with): the +// upper case form when it is one code unit, and not an ASCII letter for a non-ASCII one. +function __canon(ch) { + var u = ch.toUpperCase(); + if (u.length !== 1) return ch; + if (ch.charCodeAt(0) >= 128 && u.charCodeAt(0) < 128) return ch; + return u; +} + +// A case-insensitive `*` / `?` file mask match in time bounded by mask × name length (a +// regular expression of `.*` runs backtracks combinatorially on overlapping wildcards). +function __globMatch(mask, name) { + // Wildcards run over the original name (lowercasing it can change its length: "İ" becomes + // two code units); literal characters compare one at a time, case-insensitively as `/i` does + // ("ς" and "σ" and "Σ" match each other). + var p = mask, s = name; + var i = 0, j = 0, star = -1, mark = 0; + while (j < s.length) { + if (i < p.length && (p[i] === "?" || p[i] === s[j] || __canon(p[i]) === __canon(s[j]))) { i++; j++; } + else if (i < p.length && p[i] === "*") { star = i++; mark = j; } + else if (star !== -1) { i = star + 1; j = ++mark; } + else return false; + } + while (i < p.length && p[i] === "*") i++; + return i === p.length; +} + Folder.prototype = { constructor: Folder, get fsName() { return this.__path; }, @@ -310,10 +338,7 @@ Folder.prototype = { var out = []; for (var i = 0; i < names.length; i++) { var n = names[i]; - if (typeof mask === "string" && mask !== "*") { - var re = new RegExp("^" + mask.replace(/[.+^${}()|[\]\\]/g, "\\$&").replace(/\*/g, ".*").replace(/\?/g, ".") + "$", "i"); - if (!re.test(n.split(/[\/\\]/).pop())) continue; - } + if (typeof mask === "string" && mask !== "*" && !__globMatch(mask, n.split(/[\/\\]/).pop())) continue; var isDir = JSON.parse(__file("isDir", n)); var f = isDir ? new Folder(n) : new File(n); if (typeof mask === "function" && !mask(f)) continue; diff --git a/crates/script/src/runtime.rs b/crates/script/src/runtime.rs index 830e250e9..28e490158 100644 --- a/crates/script/src/runtime.rs +++ b/crates/script/src/runtime.rs @@ -532,8 +532,33 @@ fn native_eval_file(_: &JsValue, args: &[JsValue], ctx: &mut Context) -> JsResul r } +/// Host hooks: code a script compiles at run time (`eval`, `new Function`, …) passes the same +/// nesting check as the script itself, since the parser recurses per level and a deep string +/// built at run time would overflow the stack. +struct Hooks; + +impl boa_engine::context::HostHooks for Hooks { + fn ensure_can_compile_strings( + &self, + _realm: boa_engine::realm::Realm, + parameters: &[JsString], + body: &JsString, + _direct: bool, + _context: &mut Context, + ) -> JsResult<()> { + let mut code: String = parameters.iter().map(|p| p.to_std_string_escaped()).collect::>().join(","); + // With no parameters (`eval`) the body is the source as it stands, so a leading `#!` line + // is still its first line; with parameters it sits inside a function, where it is no comment. + if !parameters.is_empty() { + code.push('\n'); + } + code.push_str(&body.to_std_string_escaped()); + check_nesting(&code).map_err(|e| JsNativeError::syntax().with_message(e).into()) + } +} + pub(crate) fn new_context() -> Result { - let mut ctx = Context::default(); + let mut ctx = boa_engine::context::ContextBuilder::new().host_hooks(std::rc::Rc::new(Hooks)).build().map_err(|e| e.to_string())?; let limits = ctx.runtime_limits_mut(); limits.set_loop_iteration_limit(200_000_000); limits.set_recursion_limit(2000); @@ -771,19 +796,129 @@ fn run_expanded(session: &mut Session, req: &ScriptRequest) -> Outcome { /// 8 MiB thread such as the Script Console's, so deeper scripts are rejected up front. pub const MAX_NESTING: usize = 96; -/// An error when `code` nests brackets deeper than [`MAX_NESTING`] (strings, template literals -/// and comments are skipped). +/// Skip template-literal text from `*i` (just past a backtick or an interpolation's `}`). Returns +/// true when it stopped past a `${` (code follows), false at the closing backtick or the end. +fn skip_template_text(b: &[u8], i: &mut usize) -> bool { + while let Some(&d) = b.get(*i) { + match d { + b'\\' => *i += 2, + b'`' => { + *i += 1; + return false; + } + b'$' if b.get(*i + 1) == Some(&b'{') => { + *i += 2; + return true; + } + _ => *i += 1, + } + } + false +} + +/// Whether a `/` after the last significant byte `prev` (at `at` in `b`) starts a regular +/// expression literal rather than dividing: after an operator, an opening bracket, a separator, +/// a keyword that takes an expression, or at the start. After a name, a number, a string or a +/// closing bracket it divides (a regular expression after `)` is the one case this misreads). +/// `after_dot`: the word ending at `at` follows a `.` (a property name, never a keyword); a `++` +/// or `--` before the `/` is a postfix update, so it divides. +fn regex_may_start(b: &[u8], prev: u8, at: usize, after_dot: bool) -> bool { + match prev { + 0 => true, + b'+' | b'-' if at > 0 && b.get(at - 1) == Some(&prev) => false, + b'(' | b',' | b'=' | b':' | b'[' | b'!' | b'&' | b'|' | b'?' | b'{' | b';' | b'+' | b'-' | b'*' | b'%' | b'<' | b'>' | b'~' | b'^' => true, + _ if after_dot => false, + c if c.is_ascii_alphanumeric() || c == b'_' || c == b'$' => { + let start = (0..=at).rev().find(|&k| !b.get(k).is_some_and(|d| d.is_ascii_alphanumeric() || *d == b'_' || *d == b'$')).map_or(0, |k| k + 1); + let word = b.get(start..=at).unwrap_or_default(); + [&b"return"[..], b"typeof", b"instanceof", b"in", b"of", b"new", b"delete", b"void", b"throw", b"case", b"do", b"else", b"yield", b"await"] + .contains(&word) + } + _ => false, + } +} + +/// Whether the `.` at `at` is a property access: not the decimal point of a number (`1.`, `12.`) +/// that ends there. A number written `1.5.x` has a fraction before the dot, so that dot accesses. +fn dot_is_property(b: &[u8], at: usize) -> bool { + let word = |d: u8| d.is_ascii_alphanumeric() || d == b'_' || d == b'$'; + let start = (0..at).rev().find(|&k| !b.get(k).is_some_and(|d| word(*d))).map_or(0, |k| k + 1); + let before = b.get(start..at).unwrap_or_default(); + let integer = !before.is_empty() && before.iter().all(u8::is_ascii_digit); + // `1.5.x`: digits that follow another `.` are a fraction. + !(integer && !(start > 0 && b.get(start - 1) == Some(&b'.'))) +} + +/// Skip a regular expression literal that starts at `b[*i]` (a `/`), to its closing `/` (flags +/// follow as ordinary letters). A `/` inside a class `[…]` or after a backslash does not end it, +/// and neither a quote nor a backtick inside it starts a string; a line break ends it. +fn skip_regex(b: &[u8], i: &mut usize) { + *i += 1; + let mut class = false; + while let Some(&d) = b.get(*i) { + match d { + b'\\' => *i += 1, + b'[' => class = true, + b']' => class = false, + b'/' if !class => return, + b'\n' => return, + _ => {} + } + *i += 1; + } +} + +/// An error when `code` nests brackets deeper than [`MAX_NESTING`] (strings, regular expression +/// literals, template literal text and comments are skipped; the code inside `${…}` +/// interpolations is checked). pub fn check_nesting(code: &str) -> Result<(), String> { let b = code.as_bytes(); let (mut i, mut depth, mut max) = (0usize, 0usize, 0usize); + // The last significant byte (not space or comment), and where it is. + let (mut prev, mut prev_at) = (0u8, 0usize); + // A `#!` line at the very start is a comment (Boa reads it so): its text is not scanned. + if b.starts_with(b"#!") { + i = b.iter().position(|d| *d == b'\n').unwrap_or(b.len()); + } + // Whether the word that ends at `prev_at` came right after a `.` token (comments and + // whitespace between them do not matter). + let mut word_after_dot = false; + // The bracket depth of each open `${`, to tell its closing `}` from an inner one. + let mut interpolations: Vec = vec![]; while let Some(&c) = b.get(i) { match c { + b'`' => { + i += 1; + if skip_template_text(b, &mut i) { + depth += 1; + max = max.max(depth); + interpolations.push(depth); + (prev, prev_at) = (b'{', i - 1); + } else { + (prev, prev_at) = (b'`', i.saturating_sub(1)); + } + continue; + } + b'}' if interpolations.last() == Some(&depth) => { + interpolations.pop(); + depth -= 1; + i += 1; + if skip_template_text(b, &mut i) { + depth += 1; + max = max.max(depth); + interpolations.push(depth); + (prev, prev_at) = (b'{', i - 1); + } else { + (prev, prev_at) = (b'`', i.saturating_sub(1)); + } + continue; + } b'(' | b'[' | b'{' => { depth += 1; max = max.max(depth); } b')' | b']' | b'}' => depth = depth.saturating_sub(1), - b'"' | b'\'' | b'`' => { + b'"' | b'\'' => { // Skip to the closing quote (escapes skip a byte). i += 1; while let Some(&d) = b.get(i) { @@ -799,16 +934,29 @@ pub fn check_nesting(code: &str) -> Result<(), String> { while b.get(i).is_some_and(|d| *d != b'\n') { i += 1; } + continue; } b'/' if b.get(i + 1) == Some(&b'*') => { i += 2; while i < b.len() && !(b.get(i) == Some(&b'*') && b.get(i + 1) == Some(&b'/')) { i += 1; } - i += 1; + i += 2; + continue; } + b'/' if regex_may_start(b, prev, prev_at, word_after_dot) => skip_regex(b, &mut i), _ => {} } + if !c.is_ascii_whitespace() { + let word = |d: u8| d.is_ascii_alphanumeric() || d == b'_' || d == b'$'; + // A word starts where the byte before it is not a word byte (white space and comments + // end a word too): it is a property name when a `.` that is not part of a number + // comes right before it. + if word(c) && (i == 0 || !b.get(i - 1).is_some_and(|d| word(*d))) { + word_after_dot = prev == b'.' && dot_is_property(b, prev_at); + } + (prev, prev_at) = (c, i); + } i += 1; } if max > MAX_NESTING { @@ -882,6 +1030,44 @@ mod nesting_tests { assert!(check_nesting(&("(".repeat(MAX_NESTING) + &")".repeat(MAX_NESTING))).is_ok()); } + #[test] + fn regular_expression_literals_are_skipped_not_read_as_strings_or_templates() { + let deep = |n: usize| format!("{}1{}", "(".repeat(n), ")".repeat(n)); + // A backtick, a quote or a slash inside a class in a regular expression opens nothing, so + // the deep expression after it is still counted. + for re in ["/`/", "/\"/", "/'/", "/[/]/", "/\\//"] { + assert!(check_nesting(&format!("`x${{ {re}.test('x') ? {} : 0 }}`", deep(5000))).is_err(), "{re} in an interpolation"); + assert!(check_nesting(&format!("x = {re}.test(y) ? {} : 0", deep(5000))).is_err(), "{re}"); + assert!(check_nesting(&format!("x = {re}.test(y) ? {} : 0", deep(10))).is_ok(), "{re} shallow"); + } + // Division is not a regular expression: what follows it is still scanned. + assert!(check_nesting(&format!("a = 4 / 2; b = {}", deep(5000))).is_err()); + assert!(check_nesting(&format!("a = (4) / 2 / 3; b = {}", deep(5000))).is_err()); + // After a keyword that takes an expression a slash starts a regular expression. + assert!(check_nesting(&format!("typeof /`/ ; {}", deep(5000))).is_err()); + // A postfix update or a property named like a keyword is followed by a division, not a + // regular expression (the deep brackets after it must still be counted). + for head in ["i++ /", "i-- /", "obj.in /", "obj . in /", "obj . /* c */ in /", "obj.\n typeof /", "a1.in /", "1.5.in /"] { + assert!(check_nesting(&format!("x = {head} {} / 2", deep(5000))).is_err(), "{head}"); + } + // A keyword after a property name is still a keyword (`obj.in in /re/`), and the dot of a + // number does not make the word after it a property (`1. in /re/`): the regular expression + // is skipped, so the backtick inside it opens nothing and the brackets after it count. + for head in ["obj.in in", "1. in", "12. typeof"] { + assert!(check_nesting(&format!("x = {head} /`/; y = {}", deep(5000))).is_err(), "{head}"); + } + } + + #[test] + fn template_interpolations_are_checked_but_template_text_is_not() { + let deep = |n: usize| format!("`a${{{}1{}}}b`", "(".repeat(n), ")".repeat(n)); + assert!(check_nesting(&deep(5000)).is_err()); + assert!(check_nesting(&deep(10)).is_ok()); + // Brackets in the literal text, and a nested template, are fine; depth carries across `}`. + assert!(check_nesting(&format!("`{} ${{ `x${{1+(2)}}y` }} {}`", "(".repeat(500), "{".repeat(500))).is_ok()); + assert!(check_nesting(&format!("`${{1}}${{{}2{}}}`", "[".repeat(MAX_NESTING), "]".repeat(MAX_NESTING))).is_err()); + } + /// A canonical drive path and a lexical one compare once the `\\?\` prefix is gone; a UNC /// share and a Unix path are left as they are. #[test] diff --git a/crates/script/src/scriptfile.rs b/crates/script/src/scriptfile.rs index 2b6afcfbe..c3179711b 100644 --- a/crates/script/src/scriptfile.rs +++ b/crates/script/src/scriptfile.rs @@ -56,6 +56,10 @@ pub(crate) fn read(s: &Session, path: &Path) -> Result { if std::fs::metadata(path).is_ok_and(|m| m.len() > MAX_FILE_BYTES as u64) { return Err(too_big()); } + // A pipe or device reports no size and may never end: only regular files are read. + if std::fs::metadata(path).is_ok_and(|m| !m.is_file()) { + return Err(format!("Unable to read {}: not a regular file", path.display())); + } let bytes = s.services.read_file(&path.to_string_lossy()).map_err(|e| format!("Unable to read {}: {e}", path.display()))?; if bytes.len() > MAX_FILE_BYTES { return Err(too_big()); diff --git a/crates/script/src/socket.rs b/crates/script/src/socket.rs index c024fcad5..ff5d6d200 100644 --- a/crates/script/src/socket.rs +++ b/crates/script/src/socket.rs @@ -142,7 +142,7 @@ mod imp { /// Fill a stream's buffer until `done(buf)` or the timeout / end of stream. Fails only when /// the read deadline can't be set. - fn fill(s: &mut TcpStream, buf: &mut Vec, eof: &mut bool, limit: Duration, done: impl Fn(&[u8]) -> bool) -> std::io::Result<()> { + fn fill(s: &mut TcpStream, buf: &mut Vec, eof: &mut bool, limit: Duration, mut done: impl FnMut(&[u8]) -> bool) -> std::io::Result<()> { let end = Instant::now() + limit; let mut chunk = [0u8; 4096]; while !done(buf) && !*eof { @@ -322,7 +322,15 @@ mod imp { } } ("readln", Sock::Stream { s, buf, eof }) => { - if let Err(e) = fill(s, buf, eof, timeout(p), |b| b.contains(&b'\n')) { + // Only the bytes that arrived since the last look are searched: a peer + // sending no newline must not make every read rescan the whole buffer. + let mut seen = 0; + let newline = |b: &[u8]| { + let hit = b.get(seen..).is_some_and(|new| new.contains(&b'\n')); + seen = b.len(); + hit + }; + if let Err(e) = fill(s, buf, eof, timeout(p), newline) { return err(format!("cannot set the read timeout: {e}")); } let line: Vec = match buf.iter().position(|&c| c == b'\n') { diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index c7cd8730c..800ccdde7 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -2000,6 +2000,51 @@ fn setting_comp_width_and_height_leaves_the_layers_where_they_are() { assert_eq!(layer(c, "s").props.prop("transform/position").unwrap().value, KV::Vec3([100.0, 100.0, 0.0])); } +#[test] +fn a_hashbang_line_does_not_hide_deep_nesting_from_eval() { + // Boa reads a leading `#!` line of eval source as a comment, so the backtick on it opens no + // template: the brackets after it are counted and refused. + let mut s = session(); + let code = "var deep = new Array(5000).join('(') + '1' + new Array(5000).join(')'); var r; \ + try { eval('#! `\\n' + deep); r = 'ran'; } catch (e) { r = 'refused'; } r"; + assert_eq!(ok(&mut s, code).result.as_str(), Some("refused")); + assert!(crate::runtime::check_nesting(&format!("#! `\n{}1{}", "(".repeat(5000), ")".repeat(5000))).is_err()); + assert!(crate::runtime::check_nesting("#! `\n(1)").is_ok()); +} + +#[test] +fn code_compiled_at_run_time_passes_the_nesting_check() { + // `eval` and `new Function` of a string nested past the limit are refused with a catchable + // error instead of overflowing the parser's stack; shallow code still compiles. + let mut s = session(); + for call in ["eval(deep)", "new Function(deep)", "Function('a', deep)"] { + let code = format!( + "var deep = new Array(5000).join('(') + '1' + new Array(5000).join(')'); var r; try {{ {call}; r = 'ran'; }} catch (e) {{ r = 'refused'; }} r" + ); + assert_eq!(ok(&mut s, &code).result.as_str(), Some("refused"), "{call}"); + } + assert_eq!(ok(&mut s, "eval('1 + (2 * 3)') + new Function('return 4')()").result.as_f64(), Some(11.0)); +} + +#[test] +fn file_masks_match_without_backtracking() { + let mut s = session(); + let r = ok( + &mut s, + "[__globMatch('*.JSX', 'a.jsx'), __globMatch('a?c*', 'abcdef'), __globMatch('*a*b', 'xaxb'), __globMatch('*.png', 'a.jsx'), __globMatch('*a*a*a*a*a*a*a*a*a*a*a*b', new Array(60).join('a'))].join()", + ); + assert_eq!(r.result.as_str(), Some("true,true,true,false,false")); + // `?` still matches one character that lowercases to two. + let r = ok(&mut s, "[__globMatch('?.txt', '\\u0130.txt'), __globMatch('\\u0130*', '\\u0130x')].join()"); + assert_eq!(r.result.as_str(), Some("true,true")); + // Greek sigma matches in every case form, as the case-insensitive regular expression did. + let r = ok( + &mut s, + "[__globMatch('\\u03bf\\u03c2.txt', '\\u039f\\u03a3.txt'), __globMatch('\\u03a3*', '\\u03c2x'), __globMatch('k', '\\u212a'), __globMatch('a', 'b')].join()", + ); + assert_eq!(r.result.as_str(), Some("true,true,false,false")); +} + /// #3395: setValue on a string effect parameter stores the text, or a File's path, as `prop.set` /// does. Before, every string fell through to the number check ("value must be a number"). /// Hidden string parameters (Mesh Warp's Distortion Mesh) stay refused, keys included.