From 0fff075b34689f687854ef1164bf3b88de59576e Mon Sep 17 00:00:00 2001 From: Shivay-98 Date: Mon, 20 Jul 2026 21:39:29 -0700 Subject: [PATCH] net: handle undefined parent in _unrefTimer and _destroy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix and approach are from #64491 by Shivay-98; this reopens it to get it landed, since the original stalled awaiting requested changes. `Socket.prototype._unrefTimer` and `Socket.prototype._destroy` both walk the `_parent` chain with a strict `!== null` check. During connection teardown a socket's `_parent` can be left `undefined` (for example a TLS socket layered over another stream), so the loop steps onto `undefined` and reads a property off it, throwing a TypeError: Cannot read properties of undefined (reading 'Symbol(timeout)') from an uncaught I/O callback and crashing the process. Using a nullish (`!= null`) check terminates the walk on both `null` and `undefined`. [petter@hightouch.io: apply the same fix to the identical loop in `_destroy`, which the original regression test already exercised via `destroy()`; add direct unit coverage for both paths.] Fixes: https://github.com/nodejs/node/issues/64490 Refs: https://github.com/nodejs/node/pull/64491 Signed-off-by: Petter Häggholm --- lib/net.js | 8 +++- ...est-net-socket-unref-timer-parent-chain.js | 25 ++++++++++++ .../test-net-unref-timer-parent-undefined.js | 39 +++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-net-socket-unref-timer-parent-chain.js create mode 100644 test/parallel/test-net-unref-timer-parent-undefined.js diff --git a/lib/net.js b/lib/net.js index d0620449c31e7b..e5dc5a82fea213 100644 --- a/lib/net.js +++ b/lib/net.js @@ -665,7 +665,9 @@ ObjectSetPrototypeOf(Socket, stream.Duplex); // Refresh existing timeouts. Socket.prototype._unrefTimer = function _unrefTimer() { - for (let s = this; s !== null; s = s._parent) { + // `_parent` may be null; we use a loose `!= null` check in case external + // code sets it to undefined. + for (let s = this; s != null; s = s._parent) { if (s[kTimeout]) s[kTimeout].refresh(); } @@ -1026,7 +1028,9 @@ Socket.prototype._destroy = function(exception, cb) { this.connecting = false; - for (let s = this; s !== null; s = s._parent) { + // `_parent` may be null; we use a loose `!= null` check in case external + // code sets it to undefined. + for (let s = this; s != null; s = s._parent) { clearTimeout(s[kTimeout]); } diff --git a/test/parallel/test-net-socket-unref-timer-parent-chain.js b/test/parallel/test-net-socket-unref-timer-parent-chain.js new file mode 100644 index 00000000000000..36a88f1c158e59 --- /dev/null +++ b/test/parallel/test-net-socket-unref-timer-parent-chain.js @@ -0,0 +1,25 @@ +'use strict'; +const common = require('../common'); + +// Walking the `_parent` chain must stop on a nullish link, not only strict +// `null`. During connection teardown a socket's `_parent` can be left +// `undefined`, which previously caused `_unrefTimer()` and `_destroy()` to read +// a property off `undefined` and throw. +// Refs: https://github.com/nodejs/node/issues/64490 + +const assert = require('assert'); +const net = require('net'); + +{ + const socket = new net.Socket(); + socket._parent = undefined; + socket._unrefTimer(); +} + +{ + const socket = new net.Socket(); + socket._parent = undefined; + socket.on('error', common.mustNotCall()); + socket.destroy(); + assert.strictEqual(socket.destroyed, true); +} diff --git a/test/parallel/test-net-unref-timer-parent-undefined.js b/test/parallel/test-net-unref-timer-parent-undefined.js new file mode 100644 index 00000000000000..01e45e82f908e0 --- /dev/null +++ b/test/parallel/test-net-unref-timer-parent-undefined.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +// A TLS socket whose `_parent` is left `undefined` during teardown must not +// crash when reads land on it (`onStreamRead` -> `_unrefTimer`) or when it is +// destroyed (`_destroy`). Both walk the `_parent` chain. +// Refs: https://github.com/nodejs/node/issues/64490 + +const options = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), +}; + +const server = tls.createServer(options, common.mustCall((conn) => { + setTimeout(() => conn.write('x'), 50); +})); + +server.listen(0, common.mustCall(() => { + const client = tls.connect({ + port: server.address().port, + rejectUnauthorized: false, + }, common.mustCall(() => { + client._parent = undefined; + })); + + client.on('data', common.mustCall(() => { + server.close(); + client.destroy(); + })); + + client.on('error', common.mustNotCall()); +}));