fix(js): stop application JavaScript from aborting core through URL.parse, stack traces and import() (#341)

* fix(js): close URL.parse, call-site and dynamic import abort paths

Application JavaScript could panic native code inside a V8 callback, which aborts the process hosting core:

- URL.parse ran a subclass constructor or a brand setter between parsing and reading the serialization, so a nested parse left the op without one. It now constructs through the captured URL constructor.
- Error.prepareStackTrace received call sites whose cached source positions an Array.prototype setter could poison. Stacks are now always formatted by deno_core, and the hook is ignored.
- A failed dynamic import unwrapped the rejection's message. import() now always rejects.

Refs #300.

* fix(js): throw URL.parse's TypeError from a captured constructor

Author: oglassdev

crates/chunk-js/README.md

@@ -36,8 +36,10 @@ numbers, strings, booleans and bigints, `ArrayBuffer`s, typed arrays and `DataVi
 and class instances, throws `DataCloneError`, unlike in browsers (an object whose internal state can't be read from
 JavaScript, such as an iterator or a `URL`, clones as a plain object if its prototype was replaced to look like one);
 `crypto.subtle.digest` with SHA-1/256/384/512 on up to 1 MiB; ordinary `ArrayBuffer`s and typed arrays; and `console`
-methods, whose output (at most 32 messages and 16 KiB per call) the backend logs. Not available: filesystem, network,
-process, timers, Node or Deno globals, runtime imports, `Intl` and locale methods, `performance`, weak references,
+methods, whose output (at most 32 messages and 16 KiB per call) the backend logs. `URL.parse` returns a plain `URL`,
+also when called on a subclass. An error's `stack` is always a string: `Error.prepareStackTrace` is ignored, so code
+never receives call-site objects. Not available: filesystem, network, process, timers, Node or Deno globals, `import()`
+(every call rejects, including a module importing itself), `Intl` and locale methods, `performance`, weak references,
 WebAssembly, `SharedArrayBuffer` and resizable `ArrayBuffer`s. A promise that can never settle fails the call, and so
 does an unhandled rejection.
 

crates/chunk-js/src/runtime.rs

@@ -98,6 +98,15 @@ impl State {
             ),
             ..Default::default()
         });
+        // deno_core's default callback honours Error.prepareStackTrace with call sites whose
+        // methods panic once application code has modified Array.prototype.
+        runtime.v8_isolate().set_prepare_stack_trace_callback(deno_core::error::format_stack_trace);
+        // deno_core's dynamic import unwraps properties of a failed import's error, which
+        // application code controls.
+        runtime.v8_isolate().set_host_import_module_dynamically_callback(|scope, _, _, _, _| reject_import(scope));
+        runtime
+            .v8_isolate()
+            .set_host_import_module_with_phase_dynamically_callback(|scope, _, _, _, _, _| reject_import(scope));
         runtime.op_state().borrow_mut().put(None::<Capabilities>);
         runtime.op_state().borrow_mut().put(None::<crate::actions::ActionCapabilities>);
         runtime.op_state().borrow_mut().put(crate::actions::Env(Json::empty()));
@@ -205,6 +214,14 @@ impl State {
     }
 }
 
+fn reject_import<'s>(scope: &mut v8::PinScope<'s, '_>) -> Option<v8::Local<'s, v8::Promise>> {
+    let resolver = v8::PromiseResolver::new(scope)?;
+    let message = v8::String::new(scope, "Runtime imports are unavailable")?;
+    let error = v8::Exception::type_error(scope, message);
+    resolver.reject(scope, error)?;
+    Some(resolver.get_promise(scope))
+}
+
 fn js_error(error: impl std::fmt::Display) -> Error {
     Error::JavaScript(error.to_string())
 }

crates/chunk-js/src/tests.rs

@@ -533,3 +533,63 @@ fn structured_clone_reads_built_ins_by_brand_despite_modified_prototypes() {
         json!({"map": 42, "view": 0x0203, "symbols": 0, "errors": vec!["DataCloneError"; 5], "shared": true, "array": [true, 3, ["0", "2"]], "keys": ["a", "b"]})
     );
 }
+
+#[test]
+fn url_parse_survives_constructors_and_setters_that_parse_urls() {
+    let execution = run(r#"
+        class Nested extends URL { constructor(...args) { super(...args); new URL("HTTP://nested/"); } }
+        const subclass = Nested.parse("HTTP://a/x");
+        const brand = Object.getOwnPropertySymbols(new URL("http://a/"))[0];
+        let nested = false;
+        Object.defineProperty(URL.prototype, brand, {
+          set(value) {
+            Object.defineProperty(this, brand, { value });
+            if (nested) return;
+            nested = true;
+            new URL("HTTP://nested/");
+            nested = false;
+          },
+        });
+        const Native = TypeError;
+        globalThis.TypeError = function () { return {}; };
+        let missing;
+        try { URL.parse(); } catch (error) { missing = error instanceof Native; }
+        return [subclass.href, subclass instanceof Nested, URL.parse("HTTP://b/").href, URL.parse("c", "HTTP://b/x/y").href, URL.parse("nope"), missing];
+    "#)
+    .unwrap();
+    assert_eq!(value(&execution), json!(["http://a/x", false, "http://b/", "http://b/x/c", null, true]));
+}
+
+#[test]
+fn stacks_are_strings_that_ignore_prepare_stack_trace() {
+    let execution = run(r#"
+        Error.prepareStackTrace = (_, frames) => frames;
+        for (const index of [0, 1, 2]) Object.defineProperty(Array.prototype, index, { get() { throw 1; }, set() {} });
+        return new Error("boom").stack;
+    "#)
+    .unwrap();
+    let stack = value(&execution);
+    assert!(
+        stack.as_str().unwrap().starts_with("Error: boom\n    at default (chunk:deployment/build-a:4:16)"),
+        "{stack}"
+    );
+    let Err(Error::JavaScript(thrown)) = run("Error.prepareStackTrace = () => 'custom'; throw new TypeError('boom');")
+    else {
+        panic!("expected a JavaScript error");
+    };
+    assert!(thrown.starts_with("TypeError: boom\n    at default (chunk:deployment/build-a:1:"), "{thrown}");
+}
+
+#[test]
+fn imports_at_runtime_reject_without_aborting() {
+    let execution = run("try { await import(import.meta.url); } catch (error) { return error.message; }").unwrap();
+    assert_eq!(value(&execution), json!("Runtime imports are unavailable"));
+    let source = r"
+        import(import.meta.url).catch(() => {});
+        await 0;
+        Error.stackTraceLimit = 0;
+        throw Object.assign(new Error('x'), { message: 42 });
+        export default () => 1;
+    ";
+    assert!(matches!(Deployment::new("build-a".into(), source.into(), Limits::default()), Err(Error::JavaScript(_))));
+}

crates/chunk-js/src/web.js

@@ -9,6 +9,7 @@
   const log = core.ops.op_chunk_log;
   const digest = core.ops.op_chunk_digest;
   const Uint8 = Uint8Array;
+  const { TypeError } = globalThis;
   // A JavaScript clone, since V8's serializer runs host-object hooks keyed by
   // the global Symbol.for("Deno.core.hostObject") brand. It accepts an explicit
   // list of types and throws DataCloneError for anything else. Built-ins are
@@ -188,10 +189,22 @@
       return encodeInto(source, destination);
     }
   }
+  // deno_web's URL.parse runs application code, a subclass constructor or a setter on the brand,
+  // between parsing and reading the result, and its op panics if that code parses another URL.
+  const NativeURL = url.URL;
+  const canParse = NativeURL.canParse;
+  NativeURL.parse = {
+    parse(input, base = undefined) {
+      if (arguments.length === 0) throw new TypeError("Failed to execute 'URL.parse': 1 argument required");
+      input = `${input}`;
+      if (base !== undefined) base = `${base}`;
+      return canParse(input, base) ? new NativeURL(input, base) : null;
+    },
+  }.parse;
   Object.assign(globalThis, {
     TextEncoder,
     TextDecoder: encoding.TextDecoder,
-    URL: url.URL,
+    URL: NativeURL,
     URLSearchParams: url.URLSearchParams,
     atob: base64.atob,
     btoa: base64.btoa,
@@ -200,8 +213,8 @@
       return clone(value);
     },
   });
-  delete url.URL.createObjectURL;
-  delete url.URL.revokeObjectURL;
+  delete NativeURL.createObjectURL;
+  delete NativeURL.revokeObjectURL;
   const levels = ["debug", "log", "info", "warn", "error"];
   globalThis.console = Object.freeze(
     Object.fromEntries(