Skip to content

Commit d86fd2d

Browse files
net: reject invalid fds before wrapping Socket
Invalid or closed file descriptors passed to `net.Socket({ fd })` could abort the process inside libuv when a write raced with close. Validate the fd with fstat first, treat loop-private fds as already in use, and treat EBADF/ENOENT/EPERM from epoll_ctl as recoverable instead of aborting. Fixes: #63308 Signed-off-by: Sankalp Thakur <sankalphimself@gmail.com>
1 parent 6a6dee3 commit d86fd2d

4 files changed

Lines changed: 119 additions & 2 deletions

File tree

deps/uv/src/unix/core.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1090,7 +1090,23 @@ void uv__io_poll_check(uv_loop_t* loop, sigset_t* pset) {
10901090
}
10911091

10921092
int uv__fd_exists(uv_loop_t* loop, int fd) {
1093-
return (unsigned) fd < loop->nwatchers && loop->watchers[fd] != NULL;
1093+
if ((unsigned) fd < loop->nwatchers && loop->watchers[fd] != NULL)
1094+
return 1;
1095+
1096+
/* Also treat the loop's private fds as in-use. Their write ends are not
1097+
* registered in watchers[] but adopting them via uv_*_open() corrupts the
1098+
* loop and can abort later in uv__io_poll().
1099+
*/
1100+
if (fd != -1 && loop->backend_fd == fd)
1101+
return 1;
1102+
if (fd != -1 &&
1103+
(loop->signal_pipefd[0] == fd || loop->signal_pipefd[1] == fd))
1104+
return 1;
1105+
if (fd != -1 &&
1106+
(loop->async_io_watcher.fd == fd || loop->async_wfd == fd))
1107+
return 1;
1108+
1109+
return 0;
10941110
}
10951111

10961112

deps/uv/src/unix/linux.c

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1331,6 +1331,13 @@ static void uv__epoll_ctl_flush(int epollfd,
13311331
if (op != EPOLL_CTL_ADD)
13321332
abort();
13331333

1334+
/* EEXIST: already watched — retry as MOD.
1335+
* EBADF/ENOENT/EPERM: fd was closed or is not epoll-able between
1336+
* uv__io_start() and here. Ignore rather than abort the process.
1337+
*/
1338+
if (cqe->res == -EBADF || cqe->res == -ENOENT || cqe->res == -EPERM)
1339+
continue;
1340+
13341341
if (cqe->res != -EEXIST)
13351342
abort();
13361343

@@ -1423,12 +1430,35 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
14231430
if (!epoll_ctl(epollfd, op, fd, &e))
14241431
continue;
14251432

1433+
/* fd may have been closed after uv__io_start() queued this update.
1434+
* That is a recoverable application mistake; do not abort the process.
1435+
*/
1436+
if (errno == EBADF || errno == ENOENT || errno == EPERM) {
1437+
w->events = 0;
1438+
if ((unsigned) fd < loop->nwatchers && loop->watchers[fd] == w) {
1439+
loop->watchers[fd] = NULL;
1440+
assert(loop->nfds > 0);
1441+
loop->nfds--;
1442+
}
1443+
continue;
1444+
}
1445+
14261446
assert(op == EPOLL_CTL_ADD);
14271447
assert(errno == EEXIST);
14281448

14291449
/* File descriptor that's been watched before, update event mask. */
1430-
if (epoll_ctl(epollfd, EPOLL_CTL_MOD, fd, &e))
1450+
if (epoll_ctl(epollfd, EPOLL_CTL_MOD, fd, &e)) {
1451+
if (errno == EBADF || errno == ENOENT || errno == EPERM) {
1452+
w->events = 0;
1453+
if ((unsigned) fd < loop->nwatchers && loop->watchers[fd] == w) {
1454+
loop->watchers[fd] = NULL;
1455+
assert(loop->nfds > 0);
1456+
loop->nfds--;
1457+
}
1458+
continue;
1459+
}
14311460
abort();
1461+
}
14321462
}
14331463

14341464
inv.events = events;

lib/net.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ const { getOptionValue } = require('internal/options');
156156
// Lazy loaded to improve startup performance.
157157
let cluster;
158158
let dns;
159+
let fs;
159160
let BlockList;
160161
let SocketAddress;
161162
let netPromises;
@@ -198,6 +199,15 @@ function getFlags(options) {
198199

199200
function createHandle(fd, is_server) {
200201
validateInt32(fd, 'fd', 0);
202+
// Validate the descriptor is open before handing it to libuv. An invalid fd
203+
// must become a JS exception (not a process abort inside uv__io_poll when a
204+
// later write races with close). See https://github.com/nodejs/node/issues/63308.
205+
fs ??= require('fs');
206+
try {
207+
fs.fstatSync(fd);
208+
} catch (err) {
209+
throw new ErrnoException(err.errno, 'fstat');
210+
}
201211
const type = guessHandleType(fd);
202212
if (type === 'PIPE') {
203213
return new Pipe(
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const net = require('net');
6+
7+
// Invalid fds must throw a JS exception from the Socket constructor rather than
8+
// aborting the process inside libuv (see https://github.com/nodejs/node/issues/63308).
9+
10+
assert.throws(
11+
() => new net.Socket({ fd: -1 }),
12+
{
13+
code: 'ERR_OUT_OF_RANGE',
14+
name: 'RangeError',
15+
}
16+
);
17+
18+
common.runWithInvalidFD((fd) => {
19+
assert.throws(
20+
() => {
21+
new net.Socket({
22+
fd,
23+
readable: false,
24+
writable: true,
25+
});
26+
},
27+
{
28+
code: 'EBADF',
29+
syscall: 'fstat',
30+
}
31+
);
32+
});
33+
34+
// Wrapping arbitrary existing fds is unsupported on Windows.
35+
if (common.isWindows)
36+
return;
37+
38+
// Iterating arbitrary fds must not abort the process. Unsupported fds throw;
39+
// in-use libuv fds return EEXIST from open; others may open and emit errors.
40+
{
41+
let fd = 3;
42+
while (fd < 64) {
43+
try {
44+
const stream = new net.Socket({
45+
fd,
46+
readable: false,
47+
writable: true,
48+
});
49+
stream.on('error', () => {});
50+
stream.write('might crash');
51+
stream.destroy();
52+
} catch {
53+
// Expected for unsupported / invalid / already-watched descriptors.
54+
}
55+
fd += 1;
56+
}
57+
}
58+
59+
setImmediate(common.mustCall(() => {
60+
// If libuv aborted, we never reach here.
61+
}));

0 commit comments

Comments
 (0)