From 847aaae521715074e376b00ab91f6aa00740916c Mon Sep 17 00:00:00 2001 From: Demian Parkhomenko <95881717+DemianParkhomenko@users.noreply.github.com> Date: Wed, 6 Nov 2024 00:22:58 +0200 Subject: [PATCH 1/3] lib: add warning when binding inspector to public IP Add `isLoopback` function to `internal/net` module to check if a given host is a loopback address. Add a warning when binding the inspector to a public IP with an open port, as it allows external hosts to connect to the inspector. Fixes: https://github.com/nodejs/node/issues/23444 Refs: https://nodejs.org/api/cli.html#--inspecthostport --- lib/inspector.js | 12 +++++++ lib/internal/net.js | 12 +++++++ test/parallel/test-internal-net-isLoopback.js | 32 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 test/parallel/test-internal-net-isLoopback.js diff --git a/lib/inspector.js b/lib/inspector.js index b623d96b68c3f7..22bca0eeb544ed 100644 --- a/lib/inspector.js +++ b/lib/inspector.js @@ -18,6 +18,8 @@ const { ERR_INSPECTOR_NOT_WORKER, } = require('internal/errors').codes; +const { isLoopback } = require('internal/net'); + const { hasInspector } = internalBinding('config'); if (!hasInspector) throw new ERR_INSPECTOR_NOT_AVAILABLE(); @@ -171,6 +173,16 @@ function inspectorOpen(port, host, wait) { if (isUint32(port)) { validateInt32(port, 'port', 0, 65535); } + if (host && !isLoopback(host)) { + process.emitWarning( + 'Binding the inspector to a public IP with an open port is insecure, ', + 'as it allows external hosts to connect to the inspector ', + 'and perform a remote code execution attack.', + 'Documentation can be found at ' + + 'https://nodejs.org/api/cli.html#--inspecthostport', + ); + } + open(port, host); if (wait) waitForDebugger(); diff --git a/lib/internal/net.js b/lib/internal/net.js index cc616000777e4d..379a9d0c2e5e11 100644 --- a/lib/internal/net.js +++ b/lib/internal/net.js @@ -68,6 +68,17 @@ function makeSyncWrite(fd) { }; } +function isLoopback(host) { + const hostLower = host.toLowerCase(); + + return ( + hostLower === 'localhost' || + hostLower.startsWith('127.') || + hostLower.startsWith('[::1]') || + hostLower.startsWith('[0:0:0:0:0:0:0:1]') + ); +} + module.exports = { kReinitializeHandle: Symbol('kReinitializeHandle'), isIP, @@ -75,4 +86,5 @@ module.exports = { isIPv6, makeSyncWrite, normalizedArgsSymbol: Symbol('normalizedArgs'), + isLoopback, }; diff --git a/test/parallel/test-internal-net-isLoopback.js b/test/parallel/test-internal-net-isLoopback.js new file mode 100644 index 00000000000000..df63bfc262eaf6 --- /dev/null +++ b/test/parallel/test-internal-net-isLoopback.js @@ -0,0 +1,32 @@ +// Flags: --expose-internals +'use strict'; +require('../common'); +const assert = require('assert'); +const net = require('internal/net'); + +const loopback = [ + 'localhost', + '127.0.0.1', + '127.0.0.255', + '127.1.2.3', + '[::1]', + '[0:0:0:0:0:0:0:1]', +]; + +const loopbackNot = [ + 'example.com', + '192.168.1.1', + '10.0.0.1', + '255.255.255.255', + '[2001:db8::1]', + '[fe80::1]', + '8.8.8.8', +]; + +for (const address of loopback) { + assert.strictEqual(net.isLoopback(address), true); +} + +for (const address of loopbackNot) { + assert.strictEqual(net.isLoopback(address), false); +} From 9c7e306dd785522aea64aa39ed644c859bcd7da5 Mon Sep 17 00:00:00 2001 From: Demian Parkhomenko <95881717+DemianParkhomenko@users.noreply.github.com> Date: Sat, 18 Jan 2025 00:14:15 +0200 Subject: [PATCH 2/3] test: add test for inspector host warning --- lib/inspector.js | 11 ++++++----- lib/internal/net.js | 5 +++++ test/parallel/test-inspector-host-warning.js | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-inspector-host-warning.js diff --git a/lib/inspector.js b/lib/inspector.js index 22bca0eeb544ed..6d01c363dad5cd 100644 --- a/lib/inspector.js +++ b/lib/inspector.js @@ -175,11 +175,12 @@ function inspectorOpen(port, host, wait) { } if (host && !isLoopback(host)) { process.emitWarning( - 'Binding the inspector to a public IP with an open port is insecure, ', - 'as it allows external hosts to connect to the inspector ', - 'and perform a remote code execution attack.', - 'Documentation can be found at ' + - 'https://nodejs.org/api/cli.html#--inspecthostport', + 'Binding the inspector to a public IP with an open port is insecure, ' + + 'as it allows external hosts to connect to the inspector ' + + 'and perform a remote code execution attack. ' + + 'Documentation can be found at ' + + 'https://nodejs.org/api/cli.html#--inspecthostport', + 'SecurityWarning', ); } diff --git a/lib/internal/net.js b/lib/internal/net.js index 379a9d0c2e5e11..3f4c6bd944c085 100644 --- a/lib/internal/net.js +++ b/lib/internal/net.js @@ -68,6 +68,11 @@ function makeSyncWrite(fd) { }; } +/** + * https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml + * https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml + * https://www.iana.org/assignments/special-use-domain-names/special-use-domain-names.xhtml + */ function isLoopback(host) { const hostLower = host.toLowerCase(); diff --git a/test/parallel/test-inspector-host-warning.js b/test/parallel/test-inspector-host-warning.js new file mode 100644 index 00000000000000..ec4b74afaef241 --- /dev/null +++ b/test/parallel/test-inspector-host-warning.js @@ -0,0 +1,17 @@ +'use strict'; + +const common = require('../common'); +common.skipIfInspectorDisabled(); +common.skipIfWorker(); + +const inspector = require('inspector'); +inspector.open(0, '0.0.0.0', false); +common.expectWarning( + 'SecurityWarning', + 'Binding the inspector to a public IP with an open port is insecure, ' + + 'as it allows external hosts to connect to the inspector ' + + 'and perform a remote code execution attack. ' + + 'Documentation can be found at ' + + 'https://nodejs.org/api/cli.html#--inspecthostport' +); +inspector.close(); From 1f7c20a67a4a602b337150a59ae2d4671bab9def Mon Sep 17 00:00:00 2001 From: Demian Parkhomenko <95881717+DemianParkhomenko@users.noreply.github.com> Date: Tue, 18 Mar 2025 10:44:26 +0200 Subject: [PATCH 3/3] test: remove unnecessary `skipIfWorker()` call --- test/parallel/test-inspector-host-warning.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-inspector-host-warning.js b/test/parallel/test-inspector-host-warning.js index ec4b74afaef241..2bcda645e29844 100644 --- a/test/parallel/test-inspector-host-warning.js +++ b/test/parallel/test-inspector-host-warning.js @@ -2,7 +2,6 @@ const common = require('../common'); common.skipIfInspectorDisabled(); -common.skipIfWorker(); const inspector = require('inspector'); inspector.open(0, '0.0.0.0', false);