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

@@ -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;

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::<Vec<_>>().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<Context, String> {
-    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<usize> = 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]

crates/script/src/scriptfile.rs

@@ -56,6 +56,10 @@ pub(crate) fn read(s: &Session, path: &Path) -> Result<String, String> {
     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());

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<u8>, eof: &mut bool, limit: Duration, done: impl Fn(&[u8]) -> bool) -> std::io::Result<()> {
+    fn fill(s: &mut TcpStream, buf: &mut Vec<u8>, 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<u8> = match buf.iter().position(|&c| c == b'\n') {

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.