Skip to content

Commit

Permalink
src,lib,test: handle invalid stdio configuration gracefully
Browse files Browse the repository at this point in the history
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: nodejs#55932
Signed-off-by: Juan José Arboleda <[email protected]>
  • Loading branch information
juanarbol committed Nov 21, 2024
1 parent f270462 commit 20771c1
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
7 changes: 7 additions & 0 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
11 changes: 11 additions & 0 deletions src/process_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "stream_wrap.h"
#include "util-inl.h"

#include <stdio.h>
#include <climits>
#include <cstdlib>
#include <cstring>
Expand Down Expand Up @@ -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();

Expand Down
28 changes: 28 additions & 0 deletions test/parallel/test-child-process-muted-array-proto.js
Original file line number Diff line number Diff line change
@@ -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');
});

0 comments on commit 20771c1

Please sign in to comment.