src: ensure Socket(fd) cannot bypass allow-net permission

Signed-off-by: James M Snell <jasnell@gmail.com>
Assisted-by: Opencode
PR-URL: #66117
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

Author: jasnell

src/env-inl.h

@@ -322,6 +322,10 @@ inline void Environment::set_env_vars(std::shared_ptr<KVStore> env_vars) {
   env_vars_ = env_vars;
 }
 
+inline int Environment::ipc_channel_fd() const {
+  return ipc_channel_fd_;
+}
+
 inline bool Environment::printed_error() const {
   return printed_error_;
 }

src/env.cc

@@ -32,6 +32,7 @@
 
 #include <algorithm>
 #include <atomic>
+#include <charconv>
 #include <cinttypes>
 #include <cstdio>
 #include <iostream>
@@ -1044,6 +1045,21 @@ Environment::Environment(IsolateData* isolate_data,
   // which may or may not be the system environment variable store.
   enabled_debug_list_.Parse(this);
 
+  if (is_main_thread()) {
+    // setupChildProcessIpcChannel() in lib/internal/process/pre_execution.js
+    // adopts the IPC channel passed by the parent process and then removes
+    // NODE_CHANNEL_FD from the environment. Record the descriptor before any
+    // JavaScript runs, so that later changes to the environment cannot affect
+    // which descriptor is treated as the IPC channel.
+    std::optional<std::string> channel_fd = env_vars()->Get("NODE_CHANNEL_FD");
+    if (channel_fd.has_value()) {
+      int fd;
+      const char* begin = channel_fd->data();
+      auto result = std::from_chars(begin, begin + channel_fd->size(), fd);
+      if (result.ec == std::errc() && fd >= 0) ipc_channel_fd_ = fd;
+    }
+  }
+
   heap_snapshot_near_heap_limit_ =
       static_cast<uint32_t>(options_->heap_snapshot_near_heap_limit);
 

src/env.h

@@ -822,6 +822,10 @@ class Environment final : public MemoryRetainer {
   inline std::shared_ptr<KVStore> env_vars();
   inline void set_env_vars(std::shared_ptr<KVStore> env_vars);
 
+  // The IPC channel descriptor passed by the parent process through
+  // NODE_CHANNEL_FD when this Environment was created, or -1.
+  inline int ipc_channel_fd() const;
+
   inline IsolateData* isolate_data() const;
 
   inline bool printed_error() const;
@@ -1244,6 +1248,7 @@ class Environment final : public MemoryRetainer {
   permission::Permission permission_;
   const uint64_t timer_base_;
   std::shared_ptr<KVStore> env_vars_;
+  int ipc_channel_fd_ = -1;
   bool printed_error_ = false;
   bool trace_sync_io_ = false;
   bool emit_env_nonstring_warning_ = true;

src/pipe_wrap.cc

@@ -213,6 +213,14 @@ void PipeWrap::Open(const FunctionCallbackInfo<Value>& args) {
   int fd;
   if (!args[0]->Int32Value(env->context()).To(&fd)) return;
 
+  // Adopting an existing descriptor gives access to whatever it is connected
+  // to, so, like bind(), listen() and connect(), it requires the net
+  // permission.
+  if (!IsProcessStdioOrIPCChannel(env, fd)) {
+    THROW_IF_INSUFFICIENT_PERMISSIONS(
+        env, permission::PermissionScope::kNet, "");
+  }
+
   int err = uv_pipe_open(&wrap->handle_, fd);
   if (err == 0) wrap->set_fd(fd);
 

src/stream_wrap.cc

@@ -181,6 +181,9 @@ LibuvStreamWrap* LibuvStreamWrap::From(Environment* env, Local<Object> object) {
   return Unwrap<LibuvStreamWrap>(object);
 }
 
+bool LibuvStreamWrap::IsProcessStdioOrIPCChannel(Environment* env, int fd) {
+  return fd >= 0 && (fd <= 2 || fd == env->ipc_channel_fd());
+}
 
 int LibuvStreamWrap::GetFD() {
 #ifdef _WIN32

src/stream_wrap.h

@@ -103,6 +103,12 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase {
 #endif
   }
 
+  // Whether `fd` is a descriptor that the process was started with and that
+  // Node.js adopts on its behalf: one of the standard streams, backing
+  // process.stdin, process.stdout and process.stderr, or the IPC channel
+  // passed through NODE_CHANNEL_FD, backing process.send(). Adopting any other
+  // existing descriptor into a stream handle requires the net permission.
+  static bool IsProcessStdioOrIPCChannel(Environment* env, int fd);
 
  private:
   static void GetWriteQueueSize(

src/tcp_wrap.cc

@@ -369,10 +369,20 @@ void TCPWrap::Open(const FunctionCallbackInfo<Value>& args) {
   TCPWrap* wrap;
   ASSIGN_OR_RETURN_UNWRAP(
       &wrap, args.This(), args.GetReturnValue().Set(UV_EBADF));
+  Environment* env = wrap->env();
   int64_t val;
   if (!args[0]->IntegerValue(args.GetIsolate()->GetCurrentContext()).To(&val))
     return;
   int fd = static_cast<int>(val);
+
+  // Adopting an existing descriptor gives access to whatever it is connected
+  // to, so, like bind(), listen() and connect(), it requires the net
+  // permission.
+  if (!IsProcessStdioOrIPCChannel(env, fd)) {
+    THROW_IF_INSUFFICIENT_PERMISSIONS(
+        env, permission::PermissionScope::kNet, "");
+  }
+
   int err = uv_tcp_open(&wrap->handle_, fd);
 
   if (err == 0) wrap->set_fd(fd);

test/parallel/test-permission-net-cluster.js

@@ -0,0 +1,37 @@
+'use strict';
+
+// Cluster workers started with --permission and without --allow-net can use
+// their IPC channel and standard streams, which Node.js adopts without the net
+// permission.
+
+const common = require('../common');
+const assert = require('assert');
+const cluster = require('cluster');
+
+if (cluster.isPrimary) {
+  cluster.setupPrimary({
+    execArgv: ['--permission', '--allow-fs-read=*'],
+    silent: true,
+  });
+  const worker = cluster.fork();
+  let stdout = '';
+  let stderr = '';
+  worker.process.stdout.setEncoding('utf8');
+  worker.process.stdout.on('data', (chunk) => { stdout += chunk; });
+  worker.process.stderr.setEncoding('utf8');
+  worker.process.stderr.on('data', (chunk) => { stderr += chunk; });
+  worker.on('online', common.mustCall());
+  worker.on('message', common.mustCall((message) => {
+    assert.strictEqual(message, 'ready');
+    worker.disconnect();
+  }));
+  worker.process.on('close', common.mustCall((code, signal) => {
+    assert.strictEqual(signal, null);
+    assert.strictEqual(code, 0, stderr);
+    assert.strictEqual(stdout, 'stdout');
+  }));
+} else {
+  assert.strictEqual(process.permission.has('net'), false);
+  process.stdout.write('stdout');
+  process.send('ready');
+}

test/parallel/test-permission-net-socket-fd.js

@@ -0,0 +1,153 @@
+'use strict';
+
+// Adopting an existing socket descriptor, as net.Socket({ fd }) and
+// server.listen({ fd }) do, requires the net permission. The standard streams
+// and the IPC channel of the process can still be adopted without it.
+
+const common = require('../common');
+if (common.isWindows) {
+  common.skip('Socket descriptors cannot be passed through stdio on Windows');
+}
+
+const assert = require('assert');
+const { fork, spawn, spawnSync } = require('child_process');
+const net = require('net');
+const tmpdir = require('../common/tmpdir');
+
+if (process.argv[2] === 'fork-child') {
+  assert.strictEqual(process.permission.has('net'), false);
+  process.stdout.write('stdout');
+  process.once('message', common.mustCall((message) => {
+    assert.strictEqual(message, 'ping');
+    process.send('pong', () => process.disconnect());
+  }));
+  process.send('ready');
+  return;
+}
+
+// The descriptor to adopt is passed as the first argument. Changing
+// NODE_CHANNEL_FD at runtime must not make it count as the IPC channel.
+const denied = `
+  const assert = require('node:assert');
+  const net = require('node:net');
+  const fd = Number(process.argv[1]);
+  process.env.NODE_CHANNEL_FD = String(fd);
+  const expected = { code: 'ERR_ACCESS_DENIED', permission: 'Net' };
+  assert.throws(() => new net.Socket({ fd }), expected);
+  assert.throws(() => net.createServer().listen({ fd }), expected);
+  process.send?.('done');
+`;
+
+const allowed = `
+  const net = require('node:net');
+  new net.Socket({ fd: Number(process.argv[1]) }).destroy();
+`;
+
+function checkChild(execArgv, source, fd) {
+  const { status, signal, stderr } = spawnSync(
+    process.execPath,
+    [...execArgv, '--eval', source, '3'],
+    { stdio: ['ignore', 'ignore', 'pipe', fd] },
+  );
+  assert.strictEqual(signal, null);
+  assert.strictEqual(status, 0, stderr.toString());
+}
+
+tmpdir.refresh();
+
+// Connected TCP and Unix domain sockets.
+for (const options of [{ host: '127.0.0.1', port: 0 }, { path: common.PIPE }]) {
+  let client;
+  const server = net.createServer(common.mustCall((socket) => {
+    checkChild(['--permission'], denied, socket._handle.fd);
+    checkChild(['--permission', '--allow-net'], allowed, socket._handle.fd);
+    socket.destroy();
+    client.destroy();
+    server.close();
+  }));
+  server.listen(options, common.mustCall(() => {
+    const { port } = server.address();
+    client = net.connect(options.path ?? { ...options, port });
+  }));
+}
+
+// A listening TCP socket.
+{
+  const server = net.createServer(common.mustNotCall());
+  server.listen(0, '127.0.0.1', common.mustCall(() => {
+    checkChild(['--permission'], denied, server._handle.fd);
+    server.close();
+  }));
+}
+
+// The standard streams are adopted without --allow-net.
+{
+  const { status, signal, stdout, stderr } = spawnSync(process.execPath, [
+    '--permission',
+    '--eval',
+    'process.stdin.pipe(process.stdout); process.stderr.write("stderr");',
+  ], { input: 'stdin' });
+  assert.strictEqual(signal, null);
+  assert.strictEqual(status, 0, stderr.toString());
+  assert.strictEqual(stdout.toString(), 'stdin');
+  assert.strictEqual(stderr.toString(), 'stderr');
+}
+
+// fork() children get their IPC channel and standard streams without
+// --allow-net, including when the IPC channel is not on fd 3.
+for (const stdio of [
+  ['pipe', 'pipe', 'pipe', 'ipc'],
+  ['pipe', 'pipe', 'pipe', 'ignore', 'ipc'],
+]) {
+  const child = fork(__filename, ['fork-child'], {
+    execArgv: ['--permission', '--allow-fs-read=*'],
+    stdio,
+  });
+  let stdout = '';
+  let stderr = '';
+  child.stdout.setEncoding('utf8');
+  child.stdout.on('data', (chunk) => { stdout += chunk; });
+  child.stderr.setEncoding('utf8');
+  child.stderr.on('data', (chunk) => { stderr += chunk; });
+  child.on('message', common.mustCall((message) => {
+    if (message === 'ready') {
+      child.send('ping');
+    } else {
+      assert.strictEqual(message, 'pong');
+    }
+  }, 2));
+  child.on('close', common.mustCall((code, signal) => {
+    assert.strictEqual(signal, null);
+    assert.strictEqual(code, 0, stderr);
+    assert.strictEqual(stdout, 'stdout');
+  }));
+}
+
+// The IPC channel is adopted without --allow-net, but other descriptors of a
+// child with an IPC channel still require it.
+{
+  let client;
+  const server = net.createServer(common.mustCall((socket) => {
+    const child = spawn(
+      process.execPath,
+      ['--permission', '--eval', denied, '4'],
+      { stdio: ['ignore', 'ignore', 'pipe', 'ipc', socket._handle.fd] },
+    );
+    let stderr = '';
+    child.stderr.setEncoding('utf8');
+    child.stderr.on('data', (chunk) => { stderr += chunk; });
+    child.on('message', common.mustCall((message) => {
+      assert.strictEqual(message, 'done');
+    }));
+    child.on('exit', common.mustCall((code, signal) => {
+      assert.strictEqual(signal, null);
+      assert.strictEqual(code, 0, stderr);
+      socket.destroy();
+      client.destroy();
+      server.close();
+    }));
+  }));
+  server.listen(0, '127.0.0.1', common.mustCall(() => {
+    client = net.connect(server.address().port, '127.0.0.1');
+  }));
+}