From eea50f342e3090483f6da4932b84ed52bba44c58 Mon Sep 17 00:00:00 2001 From: Ludovico Fischer <43557+ludofischer@users.noreply.github.com> Date: Thu, 14 Apr 2022 16:43:16 +0200 Subject: [PATCH] fix: replace portfinder with custom implementation and fix security problem (#4384) --- lib/Server.js | 7 +- lib/getPort.js | 122 +++++++++++++++++++++++++++++++++++ package-lock.json | 89 +++---------------------- package.json | 1 - test/e2e/api.test.js | 9 ++- test/server/get-port.test.js | 36 +++++++++++ types/lib/Server.d.ts | 36 +++++++---- types/lib/getPort.d.ts | 6 ++ 8 files changed, 201 insertions(+), 105 deletions(-) create mode 100644 lib/getPort.js create mode 100644 test/server/get-port.test.js create mode 100644 types/lib/getPort.d.ts diff --git a/lib/Server.js b/lib/Server.js index 4a0604b933..9f374baef9 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -393,9 +393,8 @@ class Server { } const pRetry = require("p-retry"); - const portfinder = require("portfinder"); - - portfinder.basePort = + const getPort = require("./getPort"); + const basePort = typeof process.env.WEBPACK_DEV_SERVER_BASE_PORT !== "undefined" ? parseInt(process.env.WEBPACK_DEV_SERVER_BASE_PORT, 10) : 8080; @@ -407,7 +406,7 @@ class Server { ? parseInt(process.env.WEBPACK_DEV_SERVER_PORT_RETRY, 10) : 3; - return pRetry(() => portfinder.getPortPromise(), { + return pRetry(() => getPort(basePort), { retries: defaultPortRetry, }); } diff --git a/lib/getPort.js b/lib/getPort.js new file mode 100644 index 0000000000..b0c5080744 --- /dev/null +++ b/lib/getPort.js @@ -0,0 +1,122 @@ +"use strict"; + +/* + * Based on the packages get-port https://www.npmjs.com/package/get-port + * and portfinder https://www.npmjs.com/package/portfinder + * The code structure is similar to get-port, but it searches + * ports deterministically like portfinder + */ +const net = require("net"); +const os = require("os"); + +const minPort = 1024; +const maxPort = 65_535; + +/** + * @return {Set} + */ +const getLocalHosts = () => { + const interfaces = os.networkInterfaces(); + + // Add undefined value for createServer function to use default host, + // and default IPv4 host in case createServer defaults to IPv6. + // eslint-disable-next-line no-undefined + const results = new Set([undefined, "0.0.0.0"]); + + for (const _interface of Object.values(interfaces)) { + if (_interface) { + for (const config of _interface) { + results.add(config.address); + } + } + } + + return results; +}; + +/** + * @param {number} basePort + * @param {string | undefined} host + * @return {Promise} + */ +const checkAvailablePort = (basePort, host) => + new Promise((resolve, reject) => { + const server = net.createServer(); + server.unref(); + server.on("error", reject); + + server.listen(basePort, host, () => { + // Next line should return AdressInfo because we're calling it after listen() and before close() + const { port } = /** @type {import("net").AddressInfo} */ ( + server.address() + ); + server.close(() => { + resolve(port); + }); + }); + }); + +/** + * @param {number} port + * @param {Set} hosts + * @return {Promise} + */ +const getAvailablePort = async (port, hosts) => { + /** + * Errors that mean that host is not available. + * @type {Set} + */ + const nonExistentInterfaceErrors = new Set(["EADDRNOTAVAIL", "EINVAL"]); + /* Check if the post is available on every local host name */ + for (const host of hosts) { + try { + await checkAvailablePort(port, host); // eslint-disable-line no-await-in-loop + } catch (error) { + /* We throw an error only if the interface exists */ + if ( + !nonExistentInterfaceErrors.has( + /** @type {NodeJS.ErrnoException} */ (error).code + ) + ) { + throw error; + } + } + } + + return port; +}; + +/** + * @param {number} basePort + * @return {Promise} + */ +async function getPorts(basePort) { + if (basePort < minPort || basePort > maxPort) { + throw new Error(`Port number must lie between ${minPort} and ${maxPort}`); + } + + let port = basePort; + const hosts = getLocalHosts(); + /** @type {Set} */ + const portUnavailableErrors = new Set(["EADDRINUSE", "EACCES"]); + while (port <= maxPort) { + try { + const availablePort = await getAvailablePort(port, hosts); // eslint-disable-line no-await-in-loop + return availablePort; + } catch (error) { + /* Try next port if port is busy; throw for any other error */ + if ( + !portUnavailableErrors.has( + /** @type {NodeJS.ErrnoException} */ (error).code + ) + ) { + throw error; + } + port += 1; + } + } + + throw new Error("No available ports found"); +} + +module.exports = getPorts; diff --git a/package-lock.json b/package-lock.json index b6436e2b42..bcf46a7caf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29,7 +29,6 @@ "ipaddr.js": "^2.0.1", "open": "^8.0.9", "p-retry": "^4.5.0", - "portfinder": "^1.0.28", "rimraf": "^3.0.2", "schema-utils": "^4.0.0", "selfsigned": "^2.0.1", @@ -4105,14 +4104,6 @@ "node": ">=8" } }, - "node_modules/async": { - "version": "2.6.3", - "resolved": "https://registry.npmjs.org/async/-/async-2.6.3.tgz", - "integrity": "sha512-zflvls11DCy+dQWzTW2dzuilv8Z5X/pjfmZOWba6TNIVDm+2UDaJmXSOXlasHKfNBs8oo3M0aT50fDEWfKZjXg==", - "dependencies": { - "lodash": "^4.17.14" - } - }, "node_modules/asynckit": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", @@ -11406,7 +11397,8 @@ "node_modules/lodash": { "version": "4.17.21", "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", - "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" + "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", + "dev": true }, "node_modules/lodash.debounce": { "version": "4.0.8", @@ -11793,7 +11785,8 @@ "node_modules/minimist": { "version": "1.2.6", "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.6.tgz", - "integrity": "sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q==" + "integrity": "sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q==", + "dev": true }, "node_modules/minimist-options": { "version": "4.1.0", @@ -11818,17 +11811,6 @@ "node": ">=0.10.0" } }, - "node_modules/mkdirp": { - "version": "0.5.5", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.5.tgz", - "integrity": "sha512-NKmAlESf6jMGym1++R0Ra7wvhV+wFW63FaSOFPwRahvea0gMUcGUhVeAg/0BC0wiv9ih5NYPB1Wn1UEI1/L+xQ==", - "dependencies": { - "minimist": "^1.2.5" - }, - "bin": { - "mkdirp": "bin/cmd.js" - } - }, "node_modules/mkdirp-classic": { "version": "0.5.3", "resolved": "https://registry.npmjs.org/mkdirp-classic/-/mkdirp-classic-0.5.3.tgz", @@ -12613,27 +12595,6 @@ "node": ">=8" } }, - "node_modules/portfinder": { - "version": "1.0.28", - "resolved": "https://registry.npmjs.org/portfinder/-/portfinder-1.0.28.tgz", - "integrity": "sha512-Se+2isanIcEqf2XMHjyUKskczxbPH7dQnlMjXX6+dybayyHvAf/TCgyMRlzf/B6QDhAEFOGes0pzRo3by4AbMA==", - "dependencies": { - "async": "^2.6.2", - "debug": "^3.1.1", - "mkdirp": "^0.5.5" - }, - "engines": { - "node": ">= 0.12.0" - } - }, - "node_modules/portfinder/node_modules/debug": { - "version": "3.2.7", - "resolved": "https://registry.npmjs.org/debug/-/debug-3.2.7.tgz", - "integrity": "sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ==", - "dependencies": { - "ms": "^2.1.1" - } - }, "node_modules/postcss": { "version": "8.4.5", "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.4.5.tgz", @@ -18974,14 +18935,6 @@ "integrity": "sha512-Z7tMw1ytTXt5jqMcOP+OQteU1VuNK9Y02uuJtKQ1Sv69jXQKKg5cibLwGJow8yzZP+eAc18EmLGPal0bp36rvQ==", "dev": true }, - "async": { - "version": "2.6.3", - "resolved": "https://registry.npmjs.org/async/-/async-2.6.3.tgz", - "integrity": "sha512-zflvls11DCy+dQWzTW2dzuilv8Z5X/pjfmZOWba6TNIVDm+2UDaJmXSOXlasHKfNBs8oo3M0aT50fDEWfKZjXg==", - "requires": { - "lodash": "^4.17.14" - } - }, "asynckit": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", @@ -24399,7 +24352,8 @@ "lodash": { "version": "4.17.21", "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", - "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" + "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==", + "dev": true }, "lodash.debounce": { "version": "4.0.8", @@ -24690,7 +24644,8 @@ "minimist": { "version": "1.2.6", "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.6.tgz", - "integrity": "sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q==" + "integrity": "sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q==", + "dev": true }, "minimist-options": { "version": "4.1.0", @@ -24711,14 +24666,6 @@ } } }, - "mkdirp": { - "version": "0.5.5", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.5.tgz", - "integrity": "sha512-NKmAlESf6jMGym1++R0Ra7wvhV+wFW63FaSOFPwRahvea0gMUcGUhVeAg/0BC0wiv9ih5NYPB1Wn1UEI1/L+xQ==", - "requires": { - "minimist": "^1.2.5" - } - }, "mkdirp-classic": { "version": "0.5.3", "resolved": "https://registry.npmjs.org/mkdirp-classic/-/mkdirp-classic-0.5.3.tgz", @@ -25302,26 +25249,6 @@ } } }, - "portfinder": { - "version": "1.0.28", - "resolved": "https://registry.npmjs.org/portfinder/-/portfinder-1.0.28.tgz", - "integrity": "sha512-Se+2isanIcEqf2XMHjyUKskczxbPH7dQnlMjXX6+dybayyHvAf/TCgyMRlzf/B6QDhAEFOGes0pzRo3by4AbMA==", - "requires": { - "async": "^2.6.2", - "debug": "^3.1.1", - "mkdirp": "^0.5.5" - }, - "dependencies": { - "debug": { - "version": "3.2.7", - "resolved": "https://registry.npmjs.org/debug/-/debug-3.2.7.tgz", - "integrity": "sha512-CFjzYYAi4ThfiQvizrFQevTTXHtnCqWfe7x1AhgEscTz6ZbLbfoLRLPugTQyBth6f8ZERVUSyWHFD/7Wu4t1XQ==", - "requires": { - "ms": "^2.1.1" - } - } - } - }, "postcss": { "version": "8.4.5", "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.4.5.tgz", diff --git a/package.json b/package.json index 57ff47145e..b819330d08 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,6 @@ "ipaddr.js": "^2.0.1", "open": "^8.0.9", "p-retry": "^4.5.0", - "portfinder": "^1.0.28", "rimraf": "^3.0.2", "schema-utils": "^4.0.0", "selfsigned": "^2.0.1", diff --git a/test/e2e/api.test.js b/test/e2e/api.test.js index 791d9eb737..ab95512ba3 100644 --- a/test/e2e/api.test.js +++ b/test/e2e/api.test.js @@ -803,11 +803,10 @@ describe("API", () => { it("should throw the error when the port isn't found", async () => { expect.assertions(1); - jest.mock("portfinder", () => { - return { - getPortPromise: () => Promise.reject(new Error("busy")), - }; - }); + jest.mock( + "../../lib/getPort", + () => () => Promise.reject(new Error("busy")) + ); process.env.WEBPACK_DEV_SERVER_PORT_RETRY = 1; diff --git a/test/server/get-port.test.js b/test/server/get-port.test.js new file mode 100644 index 0000000000..b480315f00 --- /dev/null +++ b/test/server/get-port.test.js @@ -0,0 +1,36 @@ +"use strict"; + +const net = require("net"); +const util = require("util"); +const getPort = require("../../lib/getPort"); + +it("it should bind to the preferred port", async () => { + const preferredPort = 8080; + const port = await getPort(8080); + expect(port).toBe(preferredPort); +}); + +it("should pick the next port if the preferred port is unavailable", async () => { + const preferredPort = 8345; + const server = net.createServer(); + server.unref(); + await util.promisify(server.listen.bind(server))(preferredPort); + const port = await getPort(preferredPort); + expect(port).toBe(preferredPort + 1); +}); + +it("should reject privileged ports", async () => { + try { + await getPort(80); + } catch (e) { + expect(e.message).toBeDefined(); + } +}); + +it("should reject too high port numbers", async () => { + try { + await getPort(65536); + } catch (e) { + expect(e.message).toBeDefined(); + } +}); diff --git a/types/lib/Server.d.ts b/types/lib/Server.d.ts index 2695853620..4a546c6e60 100644 --- a/types/lib/Server.d.ts +++ b/types/lib/Server.d.ts @@ -685,9 +685,6 @@ declare class Server { description: string; path: string; }[]; - /** - * @type {string | undefined} - */ description: string; simpleType: string; multiple: boolean; @@ -719,9 +716,9 @@ declare class Server { description: string; multiple: boolean; path: string; - /** @type {WebSocketURL} */ type: string; + type: string; }[]; - description: string; + /** @type {ClientConfiguration} */ description: string; multiple: boolean; simpleType: string; }; @@ -846,7 +843,7 @@ declare class Server { } | { type: string; - /** @type {Object} */ values: boolean[]; + values: boolean[]; multiple: boolean; description: string; path: string; @@ -884,7 +881,7 @@ declare class Server { configs: ( | { type: string; - multiple: boolean; + /** @type {MultiCompiler} */ multiple: boolean; description: string; path: string; } @@ -1134,7 +1131,7 @@ declare class Server { description: string; negatedDescription: string; multiple: boolean; - path: string; + /** @type {ServerOptions} */ path: string; type: string; }[]; description: string; @@ -1303,7 +1300,7 @@ declare class Server { | { description: string; multiple: boolean; - path: string; + /** @type {ServerOptions} */ path: string; type: string; } )[]; @@ -2120,11 +2117,12 @@ declare class Server { } )[]; description: string; - /** @type {string} */ link: string; + link: string; }; Host: { description: string; link: string; + /** @type {ServerConfiguration} */ anyOf: ( | { enum: string[]; @@ -2263,6 +2261,12 @@ declare class Server { minLength: number; }; minItems: number; + /** + * prependEntry Method for webpack 4 + * @param {any} originalEntry + * @param {any} newAdditionalEntries + * @returns {any} + */ minLength?: undefined; } | { @@ -2346,7 +2350,7 @@ declare class Server { } | { instanceof: string; - /** @type {string} */ type?: undefined; + type?: undefined; } )[]; }; @@ -2357,7 +2361,7 @@ declare class Server { }; Server: { anyOf: { - $ref: string /** @type {MultiCompiler} */; + $ref: string; }[]; link: string; description: string; @@ -2399,6 +2403,10 @@ declare class Server { passphrase: { type: string; description: string; + /** + * @private + * @returns {Promise} + */ }; requestCert: { type: string; @@ -2791,7 +2799,7 @@ declare class Server { $ref: string; }[]; description: string; - link: string; + /** @type {Array} */ link: string; }; WebSocketServerType: { enum: string[]; @@ -2849,8 +2857,8 @@ declare class Server { bonjour: { $ref: string; }; + /** @type {any} */ client: { - /** @type {any} */ $ref: string; }; compress: { diff --git a/types/lib/getPort.d.ts b/types/lib/getPort.d.ts new file mode 100644 index 0000000000..fbe67e962f --- /dev/null +++ b/types/lib/getPort.d.ts @@ -0,0 +1,6 @@ +export = getPorts; +/** + * @param {number} basePort + * @return {Promise} + */ +declare function getPorts(basePort: number): Promise;