src: ensure Socket(fd) cannot bypass allow-net permission · nodejs/node@74835ee · GitHub
Skip to content

Commit 74835ee

Browse files
jasnelladuh95
authored andcommitted
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>
1 parent a74a0ee commit 74835ee

9 files changed

Lines changed: 242 additions & 0 deletions

File tree

‎src/env-inl.h‎

Lines changed: 4 additions & 0 deletions

‎src/env.cc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
#include <algorithm>
3434
#include <atomic>
35+
#include <charconv>
3536
#include <cinttypes>
3637
#include <cstdio>
3738
#include <iostream>
@@ -1044,6 +1045,21 @@ Environment::Environment(IsolateData* isolate_data,
10441045
// which may or may not be the system environment variable store.
10451046
enabled_debug_list_.Parse(this);
10461047

1048+
if (is_main_thread()) {
1049+
// setupChildProcessIpcChannel() in lib/internal/process/pre_execution.js
1050+
// adopts the IPC channel passed by the parent process and then removes
1051+
// NODE_CHANNEL_FD from the environment. Record the descriptor before any
1052+
// JavaScript runs, so that later changes to the environment cannot affect
1053+
// which descriptor is treated as the IPC channel.
1054+
std::optional<std::string> channel_fd = env_vars()->Get("NODE_CHANNEL_FD");
1055+
if (channel_fd.has_value()) {
1056+
int fd;
1057+
const char* begin = channel_fd->data();
1058+
auto result = std::from_chars(begin, begin + channel_fd->size(), fd);
1059+
if (result.ec == std::errc() && fd >= 0) ipc_channel_fd_ = fd;
1060+
}
1061+
}
1062+
10471063
heap_snapshot_near_heap_limit_ =
10481064
static_cast<uint32_t>(options_->heap_snapshot_near_heap_limit);
10491065

‎src/env.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,10 @@ class Environment final : public MemoryRetainer {
822822
inline std::shared_ptr<KVStore> env_vars();
823823
inline void set_env_vars(std::shared_ptr<KVStore> env_vars);
824824

825+
// The IPC channel descriptor passed by the parent process through
826+
// NODE_CHANNEL_FD when this Environment was created, or -1.
827+
inline int ipc_channel_fd() const;
828+
825829
inline IsolateData* isolate_data() const;
826830

827831
inline bool printed_error() const;
@@ -1244,6 +1248,7 @@ class Environment final : public MemoryRetainer {
12441248
permission::Permission permission_;
12451249
const uint64_t timer_base_;
12461250
std::shared_ptr<KVStore> env_vars_;
1251+
int ipc_channel_fd_ = -1;
12471252
bool printed_error_ = false;
12481253
bool trace_sync_io_ = false;
12491254
bool emit_env_nonstring_warning_ = true;

‎src/pipe_wrap.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,14 @@ void PipeWrap::Open(const FunctionCallbackInfo<Value>& args) {
213213
int fd;
214214
if (!args[0]->Int32Value(env->context()).To(&fd)) return;
215215

216+
// Adopting an existing descriptor gives access to whatever it is connected
217+
// to, so, like bind(), listen() and connect(), it requires the net
218+
// permission.
219+
if (!IsProcessStdioOrIPCChannel(env, fd)) {
220+
THROW_IF_INSUFFICIENT_PERMISSIONS(
221+
env, permission::PermissionScope::kNet, "");
222+
}
223+
216224
int err = uv_pipe_open(&wrap->handle_, fd);
217225
if (err == 0) wrap->set_fd(fd);
218226

‎src/stream_wrap.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ LibuvStreamWrap* LibuvStreamWrap::From(Environment* env, Local<Object> object) {
181181
return Unwrap<LibuvStreamWrap>(object);
182182
}
183183

184+
bool LibuvStreamWrap::IsProcessStdioOrIPCChannel(Environment* env, int fd) {
185+
return fd >= 0 && (fd <= 2 || fd == env->ipc_channel_fd());
186+
}
184187

185188
int LibuvStreamWrap::GetFD() {
186189
#ifdef _WIN32

‎src/stream_wrap.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,12 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase {
103103
#endif
104104
}
105105

106+
// Whether `fd` is a descriptor that the process was started with and that
107+
// Node.js adopts on its behalf: one of the standard streams, backing
108+
// process.stdin, process.stdout and process.stderr, or the IPC channel
109+
// passed through NODE_CHANNEL_FD, backing process.send(). Adopting any other
110+
// existing descriptor into a stream handle requires the net permission.
111+
static bool IsProcessStdioOrIPCChannel(Environment* env, int fd);
106112

107113
private:
108114
static void GetWriteQueueSize(

‎src/tcp_wrap.cc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,10 +369,20 @@ void TCPWrap::Open(const FunctionCallbackInfo<Value>& args) {
369369
TCPWrap* wrap;
370370
ASSIGN_OR_RETURN_UNWRAP(
371371
&wrap, args.This(), args.GetReturnValue().Set(UV_EBADF));
372+
Environment* env = wrap->env();
372373
int64_t val;
373374
if (!args[0]->IntegerValue(args.GetIsolate()->GetCurrentContext()).To(&val))
374375
return;
375376
int fd = static_cast<int>(val);
377+
378+
// Adopting an existing descriptor gives access to whatever it is connected
379+
// to, so, like bind(), listen() and connect(), it requires the net
380+
// permission.
381+
if (!IsProcessStdioOrIPCChannel(env, fd)) {
382+
THROW_IF_INSUFFICIENT_PERMISSIONS(
383+
env, permission::PermissionScope::kNet, "");
384+
}
385+
376386
int err = uv_tcp_open(&wrap->handle_, fd);
377387

378388
if (err == 0) wrap->set_fd(fd);
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
3+
// Cluster workers started with --permission and without --allow-net can use
4+
// their IPC channel and standard streams, which Node.js adopts without the net
5+
// permission.
6+
7+
const common = require('../common');
8+
const assert = require('assert');
9+
const cluster = require('cluster');
10+
11+
if (cluster.isPrimary) {
12+
cluster.setupPrimary({
13+
execArgv: ['--permission', '--allow-fs-read=*'],
14+
silent: true,
15+
});
16+
const worker = cluster.fork();
17+
let stdout = '';
18+
let stderr = '';
19+
worker.process.stdout.setEncoding('utf8');
20+
worker.process.stdout.on('data', (chunk) => { stdout += chunk; });
21+
worker.process.stderr.setEncoding('utf8');
22+
worker.process.stderr.on('data', (chunk) => { stderr += chunk; });
23+
worker.on('online', common.mustCall());
24+
worker.on('message', common.mustCall((message) => {
25+
assert.strictEqual(message, 'ready');
26+
worker.disconnect();
27+
}));
28+
worker.process.on('close', common.mustCall((code, signal) => {
29+
assert.strictEqual(signal, null);
30+
assert.strictEqual(code, 0, stderr);
31+
assert.strictEqual(stdout, 'stdout');
32+
}));
33+
} else {
34+
assert.strictEqual(process.permission.has('net'), false);
35+
process.stdout.write('stdout');
36+
process.send('ready');
37+
}
Lines changed: 153 additions & 0 deletions

0 commit comments

Comments
 (0)