From e079e6e5b895b83f081804cc46f0b6cfaa904219 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= <soyjuanarbol@gmail.com>
Date: Wed, 20 Nov 2024 23:46:19 -0500
Subject: [PATCH] src,lib: handle invalid stdio configuration gracefully
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes an issue where malformed or unexpected stdio configurations
could cause crashes or undefined behavior during child process
spawning. This patch ensures robust validation of stdio entries:

Fixes: https://github.com/nodejs/node/issues/55932
Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com>
---
 lib/internal/child_process.js                 |  7 +++++
 src/process_wrap.cc                           | 11 ++++++++
 .../test-child-process-muted-array-proto.js   | 28 +++++++++++++++++++
 3 files changed, 46 insertions(+)
 create mode 100644 test/parallel/test-child-process-muted-array-proto.js

diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
index 748f14a283f05f..24894624487ea7 100644
--- a/lib/internal/child_process.js
+++ b/lib/internal/child_process.js
@@ -427,6 +427,13 @@ ChildProcess.prototype.spawn = function(options) {
 
   for (i = 0; i < stdio.length; i++) {
     const stream = stdio[i];
+    // Never trust the user input/runtime
+    // Refs: https://github.com/nodejs/node/issues/55932
+    if (!stream) {
+      process.nextTick(onErrorNT, this, UV_EINVAL);
+      return UV_EINVAL;
+    }
+
     if (stream.type === 'ignore') continue;
 
     if (stream.ipc) {
diff --git a/src/process_wrap.cc b/src/process_wrap.cc
index 27a294eb384960..8ae3cc4c00518f 100644
--- a/src/process_wrap.cc
+++ b/src/process_wrap.cc
@@ -26,6 +26,7 @@
 #include "stream_wrap.h"
 #include "util-inl.h"
 
+#include <stdio.h>
 #include <climits>
 #include <cstdlib>
 #include <cstring>
@@ -122,6 +123,16 @@ class ProcessWrap : public HandleWrap {
     for (uint32_t i = 0; i < len; i++) {
       Local<Object> stdio =
           stdios->Get(context, i).ToLocalChecked().As<Object>();
+
+      // Never trust the user.
+      // Refs: https://github.com/nodejs/node/issues/55932
+      if (!stdio->IsObject()) {
+        stdio = Object::New(env->isolate());
+        stdio->Set(context,
+                   env->type_string(),
+                   env->ignore_string()).FromJust();
+      }
+
       Local<Value> type =
           stdio->Get(context, env->type_string()).ToLocalChecked();
 
diff --git a/test/parallel/test-child-process-muted-array-proto.js b/test/parallel/test-child-process-muted-array-proto.js
new file mode 100644
index 00000000000000..f10618e8c8abee
--- /dev/null
+++ b/test/parallel/test-child-process-muted-array-proto.js
@@ -0,0 +1,28 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('node:assert');
+const { spawn } = require('node:child_process');
+
+// The UNIX version of this test is enough.
+if (common.isWindows) {
+  common.skip('This test is enough for Unix-like systems');
+  return;
+}
+
+// Refs: https://github.com/nodejs/node/issues/55932
+Object.defineProperty(Array.prototype, '0', {
+  set() {
+    console.log(123);
+  },
+  get() {
+    return 123;
+  }
+}, { configurable: true, writable: true });
+
+const cp = spawn('ls');
+
+// Can't use common.mustCall() here because array prototype is mutated.
+cp.on('error', (err) => {
+  assert.strictEqual(err.code, 'EINVAL');
+});