From c32c889c450f6e428e822294ed4caeace389506d Mon Sep 17 00:00:00 2001 From: Dirceu Pereira Tiegs Date: Tue, 15 Mar 2016 20:34:19 -0300 Subject: [PATCH] net: Validate port in createServer().listen() Make sure we validate the port number in all kinds of `listen()` calls. Fixes: https://github.com/nodejs/node/issues/5727 PR-URL: https://github.com/nodejs/node/pull/5732 Reviewed-By: Evan Lucas Reviewed-By: Colin Ihrig Reviewed-By: Trevor Norris Reviewed-By: James M Snell --- lib/internal/net.js | 8 +++++- lib/net.js | 7 ++--- test/parallel/test-net-listen-port-option.js | 2 +- test/parallel/test-regress-GH-5727.js | 27 ++++++++++++++++++++ test/sequential/test-net-server-address.js | 15 ----------- 5 files changed, 39 insertions(+), 20 deletions(-) create mode 100644 test/parallel/test-regress-GH-5727.js diff --git a/lib/internal/net.js b/lib/internal/net.js index 30bd50ce93..d19bc4c219 100644 --- a/lib/internal/net.js +++ b/lib/internal/net.js @@ -1,6 +1,6 @@ 'use strict'; -module.exports = { isLegalPort }; +module.exports = { isLegalPort, assertPort }; // Check that the port number is not NaN when coerced to a number, // is an integer and that it falls within the legal range of port numbers. @@ -10,3 +10,9 @@ function isLegalPort(port) { return false; return +port === (+port >>> 0) && port <= 0xFFFF; } + + +function assertPort(port) { + if (typeof port !== 'undefined' && !isLegalPort(port)) + throw new RangeError('"port" argument must be >= 0 and < 65536'); +} diff --git a/lib/net.js b/lib/net.js index 3c87ac29d2..c2b3f5500a 100644 --- a/lib/net.js +++ b/lib/net.js @@ -24,6 +24,7 @@ var cluster; const errnoException = util._errnoException; const exceptionWithHostPort = util._exceptionWithHostPort; const isLegalPort = internalNet.isLegalPort; +const assertPort = internalNet.assertPort; function noop() {} @@ -1352,9 +1353,7 @@ Server.prototype.listen = function() { (typeof h.port === 'undefined' && 'port' in h)) { // Undefined is interpreted as zero (random port) for consistency // with net.connect(). - if (typeof h.port !== 'undefined' && !isLegalPort(h.port)) - throw new RangeError('"port" option should be >= 0 and < 65536: ' + - h.port); + assertPort(h.port); if (h.host) listenAfterLookup(h.port | 0, h.host, backlog, h.exclusive); else @@ -1375,10 +1374,12 @@ Server.prototype.listen = function() { typeof arguments[1] === 'function' || typeof arguments[1] === 'number') { // The first argument is the port, no IP given. + assertPort(port); listen(self, null, port, 4, backlog); } else { // The first argument is the port, the second an IP. + assertPort(port); listenAfterLookup(port, arguments[1], backlog); } diff --git a/test/parallel/test-net-listen-port-option.js b/test/parallel/test-net-listen-port-option.js index a2917e9508..18b256c973 100644 --- a/test/parallel/test-net-listen-port-option.js +++ b/test/parallel/test-net-listen-port-option.js @@ -18,7 +18,7 @@ net.Server().listen({ port: '' + common.PORT }, close); ].forEach(function(port) { assert.throws(function() { net.Server().listen({ port: port }, assert.fail); - }, /"port" option should be >= 0 and < 65536/i); + }, /"port" argument must be >= 0 and < 65536/i); }); [null, true, false].forEach(function(port) { diff --git a/test/parallel/test-regress-GH-5727.js b/test/parallel/test-regress-GH-5727.js new file mode 100644 index 0000000000..d481f702ed --- /dev/null +++ b/test/parallel/test-regress-GH-5727.js @@ -0,0 +1,27 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +const invalidPort = -1 >>> 0; +const errorMessage = /"port" argument must be \>= 0 and \< 65536/; + +net.Server().listen(common.PORT, function() { + assert.equal(this._connectionKey, '6::::' + common.PORT); + this.close(); +}); + +// The first argument is a configuration object +assert.throws(() => { + net.Server().listen({ port: invalidPort }, common.fail); +}, errorMessage); + +// The first argument is the port, no IP given. +assert.throws(() => { + net.Server().listen(invalidPort, common.fail); +}, errorMessage); + +// The first argument is the port, the second an IP. +assert.throws(() => { + net.Server().listen(invalidPort, '0.0.0.0', common.fail); +}, errorMessage); diff --git a/test/sequential/test-net-server-address.js b/test/sequential/test-net-server-address.js index ddc65c2df2..e0c5b08be2 100644 --- a/test/sequential/test-net-server-address.js +++ b/test/sequential/test-net-server-address.js @@ -87,18 +87,3 @@ server3.listen(0, function() { assert.strictEqual(address.family, family_ipv6); server3.close(); }); - -// Test without hostname, but with port -1 -var server4 = net.createServer(); - -server4.on('error', function(e) { - console.log('Error on ip socket: ' + e.toString()); -}); - -// Specify -1 as port number -server4.listen(-1, function() { - var address = server4.address(); - assert.strictEqual(address.address, anycast_ipv6); - assert.strictEqual(address.family, family_ipv6); - server4.close(); -});