diff --git a/changelog.d/9568-child-process-null-bytes.md b/changelog.d/9568-child-process-null-bytes.md new file mode 100644 index 0000000000..87626eef03 --- /dev/null +++ b/changelog.d/9568-child-process-null-bytes.md @@ -0,0 +1,11 @@ +### Fixed + +- **`child_process` now rejects embedded null bytes synchronously through both + direct imports and CommonJS-default namespaces (#9537).** The CJS method-call + dispatcher bypassed codegen-only validators, so `spawn`, `spawnSync`, + `execFile`, and related calls handed invalid file/argument strings to the OS + and later reported `UNKNOWN`. Runtime entry points now validate command, + file, and indexed argument strings before constructing a process, and + OS-facing `cwd`, `argv0`, `shell`, and environment strings use Node's exact + `ERR_INVALID_ARG_VALUE` message, including the property name and escaped + received value. diff --git a/crates/perry-runtime/src/child_process/exec.rs b/crates/perry-runtime/src/child_process/exec.rs index f0f22016ce..adfc1ac7bc 100644 --- a/crates/perry-runtime/src/child_process/exec.rs +++ b/crates/perry-runtime/src/child_process/exec.rs @@ -42,6 +42,7 @@ pub extern "C" fn js_child_process_exec_sync( let cmd_bytes = std::slice::from_raw_parts(data_ptr, len); String::from_utf8_lossy(cmd_bytes).into_owned() }; + validate::cp_validate_no_null_bytes("command", &cmd_str); // Execute the command using the shell, honoring `cwd`/`env` options. #[cfg(unix)] @@ -92,6 +93,8 @@ pub extern "C" fn js_child_process_spawn_sync( let cmd_data = (cmd_ptr as *const u8).add(std::mem::size_of::()); String::from_utf8_lossy(std::slice::from_raw_parts(cmd_data, cmd_len)).into_owned() }; + validate::cp_validate_no_null_bytes("file", &cmd_str); + unsafe { validate::cp_validate_raw_args(args_ptr as i64) }; let opts_val = cp_options_from_raw_args(args_ptr as i64, options_ptr as i64); let mode = cp_read_output_mode(opts_val, false); @@ -258,6 +261,7 @@ pub extern "C" fn js_child_process_exec(cmd_ptr: *const StringHeader, arg1: f64, let cmd_bytes = std::slice::from_raw_parts(data_ptr, len); String::from_utf8_lossy(cmd_bytes).into_owned() }; + validate::cp_validate_no_null_bytes("command", &cmd_str); if abort_signal.is_some_and(cp_abort_signal_is_aborted) { let stdout_box = cp_box_output(b"", &mode); @@ -338,6 +342,8 @@ pub extern "C" fn js_child_process_exec_file( }; let file_str = unsafe { cp_read_string_header(file_ptr) }; + validate::cp_validate_no_null_bytes("file", &file_str); + validate::cp_validate_args(args_val); let arg_strs = cp_args_from_value(args_val); // execFile defaults to utf8 (callback stdout/stderr are strings). let mode = cp_read_output_mode(opts_val, true); @@ -394,6 +400,8 @@ pub extern "C" fn js_child_process_exec_file_sync( opts_val: f64, ) -> f64 { let file_str = unsafe { cp_read_string_header(file_ptr) }; + validate::cp_validate_no_null_bytes("file", &file_str); + validate::cp_validate_args(args_val); let mode = cp_read_output_mode(opts_val, false); if file_str.is_empty() { return cp_box_output(b"", &mode); @@ -483,6 +491,7 @@ fn cp_promisified_run(command: Command, cmd_str: String, opts: f64) -> f64 { } extern "C" fn cp_promisified_exec(_closure: *const ClosureHeader, cmd_val: f64, opts: f64) -> f64 { + validate::cp_validate_command(cmd_val, "command"); let cmd = cp_value_to_string(cmd_val).unwrap_or_default(); #[cfg(unix)] let mut command = { @@ -508,6 +517,8 @@ extern "C" fn cp_promisified_exec_file( file_val: f64, args_val: f64, ) -> f64 { + validate::cp_validate_command(file_val, "file"); + validate::cp_validate_args(args_val); let file = cp_value_to_string(file_val).unwrap_or_default(); let arg_strs = cp_args_from_value(args_val); // The 2-arg promisify(execFile) wrapper has no options slot; resolve a bare diff --git a/crates/perry-runtime/src/child_process/options.rs b/crates/perry-runtime/src/child_process/options.rs index 41fe9cf91a..2fd591a54b 100644 --- a/crates/perry-runtime/src/child_process/options.rs +++ b/crates/perry-runtime/src/child_process/options.rs @@ -52,6 +52,7 @@ pub(crate) fn cp_apply_options(command: &mut Command, opts_val: f64) { } if let Some(dir) = cp_value_to_string(cp_get_field(opts_val, b"cwd")) { + validate::cp_validate_path_no_null_bytes("options.cwd", &dir); if !dir.is_empty() { command.current_dir(dir); } @@ -72,7 +73,11 @@ pub(crate) fn cp_apply_options(command: &mut Command, opts_val: f64) { if JSValue::from_bits(v.to_bits()).is_undefined() { continue; // Node omits keys whose value is `undefined`. } - command.env(&key, cp_coerce_string(v)); + let property = format!("options.env['{key}']"); + validate::cp_validate_no_null_bytes(&property, &key); + let value = cp_coerce_string(v); + validate::cp_validate_no_null_bytes(&property, &value); + command.env(&key, value); } } } @@ -82,7 +87,9 @@ pub(crate) fn cp_apply_options(command: &mut Command, opts_val: f64) { pub(crate) fn cp_read_argv0(opts_val: f64) -> Option { cp_object_ptr(opts_val)?; - cp_value_to_string(cp_get_field(opts_val, b"argv0")) + let argv0 = cp_value_to_string(cp_get_field(opts_val, b"argv0"))?; + validate::cp_validate_no_null_bytes("options.argv0", &argv0); + Some(argv0) } pub(crate) fn cp_read_abort_signal(opts_val: f64) -> Option { @@ -513,7 +520,10 @@ pub(crate) fn cp_build_command(cmd: &str, args: &[String], opts_val: f64) -> Com let mut command = if crate::value::js_is_truthy(shell) != 0 { // `shell: ""` picks the binary; `shell: true` uses the default. let shell_bin = match cp_value_to_string(shell) { - Some(s) if !s.is_empty() => s, + Some(s) if !s.is_empty() => { + validate::cp_validate_no_null_bytes("options.shell", &s); + s + } _ => cp_default_shell(), }; let mut line = program.clone(); diff --git a/crates/perry-runtime/src/child_process/reactor.rs b/crates/perry-runtime/src/child_process/reactor.rs index 9d54b0302c..65cda0298e 100644 --- a/crates/perry-runtime/src/child_process/reactor.rs +++ b/crates/perry-runtime/src/child_process/reactor.rs @@ -1041,6 +1041,8 @@ pub extern "C" fn js_child_process_spawn_streams( cp_read_arg_strings(args_ptr), ) }; + validate::cp_validate_no_null_bytes("file", &cmd_str); + unsafe { validate::cp_validate_raw_args(args_ptr) }; // `opts_ptr` arrives as a raw (unboxed) heap pointer; re-box it so the // options helpers can read `cwd`/`env`/`shell`. Small values mean diff --git a/crates/perry-runtime/src/child_process/validate.rs b/crates/perry-runtime/src/child_process/validate.rs index d1314fb3f5..91d059fe5e 100644 --- a/crates/perry-runtime/src/child_process/validate.rs +++ b/crates/perry-runtime/src/child_process/validate.rs @@ -13,15 +13,82 @@ use crate::value::JSValue; use super::{ - cp_array_ptr, cp_get_field, cp_object_ptr, cp_signal_is_valid, cp_stdio_stream_fd, + cp_array_ptr, cp_box_ptr, cp_get_field, cp_object_ptr, cp_signal_is_valid, cp_stdio_stream_fd, cp_value_to_string, }; -fn cp_throw_null_bytes() -> ! { - crate::fs::validate::throw_type_error_with_code( - "The argument must not contain null bytes", - "ERR_INVALID_ARG_VALUE", - ); +fn cp_inspect_received_string(received: &str) -> String { + // Node's ERR_INVALID_ARG_VALUE uses util.inspect's string quoting: prefer + // a delimiter absent from the value, and escape controls/backslashes. + let quote = if !received.contains('\'') { + '\'' + } else if !received.contains('"') { + '"' + } else if !received.contains('`') && !received.contains("${") { + '`' + } else { + '\'' + }; + let mut display = String::with_capacity(received.len() + 2); + display.push(quote); + for ch in received.chars() { + match ch { + '\0' => display.push_str("\\x00"), + '\x08' => display.push_str("\\b"), + '\x0c' => display.push_str("\\f"), + '\n' => display.push_str("\\n"), + '\r' => display.push_str("\\r"), + '\t' => display.push_str("\\t"), + '\x0b' => display.push_str("\\v"), + '\\' => display.push_str("\\\\"), + ch if ch == quote => { + display.push('\\'); + display.push(ch); + } + ch if ch.is_ascii_control() => { + use std::fmt::Write; + let _ = write!(display, "\\x{:02x}", ch as u32); + } + ch => display.push(ch), + } + } + display.push(quote); + display +} + +fn cp_throw_null_bytes(name: &str, received: &str, path_like: bool) -> ! { + let subject = if name.starts_with("options.") { + "property" + } else { + "argument" + }; + let expected = if path_like { + "a string, Uint8Array, or URL without null bytes" + } else { + "a string without null bytes" + }; + let display = cp_inspect_received_string(received); + let message = format!("The {subject} '{name}' must be {expected}. Received {display}"); + crate::fs::validate::throw_type_error_with_code(&message, "ERR_INVALID_ARG_VALUE"); +} + +/// Reject a string before it crosses into `std::process::Command`. This lives +/// in the runtime, rather than only in the direct-import lowering, so CJS +/// namespace dispatch and future call paths cannot bypass the check. Node's +/// `ERR_INVALID_ARG_VALUE` distinguishes top-level arguments from `options.*` +/// properties in the message. +pub(super) fn cp_validate_no_null_bytes(name: &str, received: &str) { + if received.contains('\0') { + cp_throw_null_bytes(name, received, false); + } +} + +/// `options.cwd` is PathLike, so Node's expected-type clause also names +/// Uint8Array and URL even when the received value is a string. +pub(super) fn cp_validate_path_no_null_bytes(name: &str, received: &str) { + if received.contains('\0') { + cp_throw_null_bytes(name, received, true); + } } /// Validate a `command` / `file` argument. `value` is the original NaN-boxed @@ -31,8 +98,8 @@ fn cp_throw_null_bytes() -> ! { /// message when `value` is not a string. A no-op for any string. pub(crate) fn cp_validate_command(value: f64, name: &str) { if JSValue::from_bits(value.to_bits()).is_any_string() { - if cp_value_to_string(value).is_some_and(|value| value.contains('\0')) { - cp_throw_null_bytes(); + if let Some(received) = cp_value_to_string(value) { + cp_validate_no_null_bytes(name, &received); } return; } @@ -48,7 +115,7 @@ pub(crate) fn cp_validate_command(value: f64, name: &str) { /// (arrays and plain objects), but throws `TypeError [ERR_INVALID_ARG_TYPE]` /// with `The "args" argument must be of type object. Received …` for a /// primitive such as a string, number, boolean, bigint, or symbol. -fn cp_validate_args(value: f64) { +pub(super) fn cp_validate_args(value: f64) { let jv = JSValue::from_bits(value.to_bits()); // Node's `normalizeSpawnArguments` / `normalizeExecFileArgs` reject the // args slot only when it is a non-nullish *primitive* (string, number, @@ -67,8 +134,8 @@ fn cp_validate_args(value: f64) { if let Some(args) = cp_array_ptr(value) { for i in 0..unsafe { (*args).length } { let value = crate::array::js_array_get_f64(args, i); - if cp_value_to_string(value).is_some_and(|value| value.contains('\0')) { - cp_throw_null_bytes(); + if let Some(received) = cp_value_to_string(value) { + cp_validate_no_null_bytes(&format!("args[{i}]"), &received); } } } @@ -81,6 +148,21 @@ fn cp_validate_args(value: f64) { crate::fs::validate::throw_type_error_with_code(&message, "ERR_INVALID_ARG_TYPE"); } +/// Validate a raw args slot after codegen/native-module dispatch has stripped +/// its NaN-box tag. Only real heap pointers can name an args array; nullish and +/// options-overload sentinels are intentionally ignored. Re-boxing lets the +/// shared validator preserve the original array index in its error message. +/// +/// # Safety +/// +/// A non-sentinel `args_ptr` must be a runtime-managed heap pointer. +pub(super) unsafe fn cp_validate_raw_args(args_ptr: i64) { + if args_ptr <= 0 || !crate::value::addr_class::is_above_handle_band(args_ptr as usize) { + return; + } + cp_validate_args(cp_box_ptr(args_ptr as *const u8)); +} + fn cp_throw_option_type(name: &str, value: f64) -> ! { let message = format!( "The \"options.{name}\" property must be of type {}. Received {}", diff --git a/test-files/test_gap_9537_child_process_null_bytes.ts b/test-files/test_gap_9537_child_process_null_bytes.ts new file mode 100644 index 0000000000..910b02983f --- /dev/null +++ b/test-files/test_gap_9537_child_process_null_bytes.ts @@ -0,0 +1,55 @@ +// #9537 — every child_process spelling must reject embedded null bytes +// synchronously, before a command reaches the OS. The direct-import lowering +// already called setup validators, but CJS-default namespace dispatch skipped +// them and surfaced an asynchronous `UNKNOWN` spawn error instead. +// +// Byte-for-byte vs `node --experimental-strip-types`. +import * as direct from "node:child_process"; +import { createRequire } from "node:module"; + +const require = createRequire(import.meta.url); +const cjs: any = require("child_process"); + +function probe(label: string, run: () => any): void { + try { + const value = run(); + // Keep the broken async-spawn path from turning the fixture's intended + // mismatch into an unhandled `error` event on a pre-fix binary. + if (value && typeof value.on === "function") value.on("error", () => {}); + console.log(label, "NO_THROW"); + } catch (e: any) { + console.log(label, "|", e.name, "|", e.code, "|", e.message); + } +} + +// Existing direct-import path: pin the corrected Node message, including the +// argument name, original array index, escaped received value, class and code. +probe("direct spawn file", () => direct.spawn("/bin/tr\x00ue", [])); +probe("direct spawn args[1]", () => direct.spawn("/bin/true", ["ok", "a\x00b"])); +probe("direct spawn escaped received", () => + direct.spawn("/bin/true", ["a'\\\n\x00b"])); +probe("direct exec command", () => direct.exec("echo\x00x", () => {})); +probe("direct execFileSync args[1]", () => + direct.execFileSync("/bin/true", ["ok", "a\x00b"])); + +// Regression path: fused method calls on require("child_process") route +// through the native-module dispatcher and then the runtime entry points. +probe("cjs spawn file", () => cjs.spawn("/bin/tr\x00ue", [])); +probe("cjs spawn args[1]", () => cjs.spawn("/bin/true", ["ok", "a\x00b"])); +probe("cjs spawnSync args[1]", () => cjs.spawnSync("/bin/true", ["ok", "a\x00b"])); +probe("cjs exec command", () => cjs.exec("echo\x00x", () => {})); +probe("cjs execSync command", () => cjs.execSync("echo\x00x")); +probe("cjs execFile args[1]", () => + cjs.execFile("/bin/true", ["ok", "a\x00b"], () => {})); +probe("cjs execFileSync file", () => cjs.execFileSync("/bin/tr\x00ue", [])); + +// OS-facing option strings use Node's `property` wording. `cwd` is PathLike, +// so its expected-type clause additionally names Uint8Array and URL. +probe("cjs cwd", () => cjs.spawn("/bin/true", [], { cwd: "/tmp/a\x00b" })); +probe("cjs argv0", () => cjs.spawn("/bin/true", [], { argv0: "a\x00b" })); +probe("cjs shell", () => cjs.spawn("true", [], { shell: "/bin/s\x00h" })); +probe("cjs env value", () => cjs.spawn("/bin/true", [], { env: { A: "a\x00b" } })); +probe("cjs env undefined key", () => + cjs.spawn("/bin/true", [], { env: { ["A\x00B"]: undefined } })); + +console.log("done");