Skip to content

lib: add warning when binding inspector to public IP #55736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions lib/inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -171,6 +173,17 @@ 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',
'SecurityWarning',
);
}

open(port, host);
if (wait)
waitForDebugger();
Expand Down
17 changes: 17 additions & 0 deletions lib/internal/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,28 @@ 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();

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,
isIPv4,
isIPv6,
makeSyncWrite,
normalizedArgsSymbol: Symbol('normalizedArgs'),
isLoopback,
};
16 changes: 16 additions & 0 deletions test/parallel/test-inspector-host-warning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict';

const common = require('../common');
common.skipIfInspectorDisabled();

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();
32 changes: 32 additions & 0 deletions test/parallel/test-internal-net-isLoopback.js
Original file line number Diff line number Diff line change
@@ -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',
Comment on lines +18 to +19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we really warn for these? IMHO binding to IP in private subnet is usually fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition when to warn was discussed in #23756 (review) and #23756 (comment). So, I think we should warn for all non-loopback addresses. WDYT?

'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);
}
Loading