Skip to content

Fix security issues reported by Dependabot for version 4 #5514

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

Draft
wants to merge 3 commits into
base: version-4
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 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
46 changes: 39 additions & 7 deletions lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -2014,7 +2014,8 @@ class Server {
this.checkHeader(
/** @type {{ [key: string]: string | undefined }} */
(req.headers),
"host"
"host",
true
)
) {
return next();
Expand Down Expand Up @@ -2208,6 +2209,32 @@ class Server {
this.options.onBeforeSetupMiddleware(this);
}

// Register setup cross origin request check for securityAdd commentMore actions
middlewares.push({
name: "cross-origin-header-check",
/**
* @param {Request} req
* @param {Response} res
* @param {NextFunction} next
* @returns {void}
*/
middleware: (req, res, next) => {
const headers =
/** @type {{ [key: string]: string | undefined }} */
(req.headers);
if (
Copy link
Member

Choose a reason for hiding this comment

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

Recently we added some fixes here to skip check for allowedHost, please add it here

Copy link
Author

@kretajak kretajak Jun 9, 2025

Choose a reason for hiding this comment

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

It seems not that straightforward. To me bigger effort is needed to incorporate changes from 03d1214. This PR uses functions defined in previous commits not available in line 4.

If it's not a problem, I would consider merging this PR and creating separate PR for task you mentioned.

Copy link
Member

Choose a reason for hiding this comment

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

@kretajak let's include 03d1214 here, otherwise in some cases it will be impossible to setup a new version and we can merge

Copy link

Choose a reason for hiding this comment

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

@kretajak let me know if you need help backporting that commit. I could try to add it on top of your existing PR if you don't have time to. We have at least 3 Docusaurus issues asking us to solve this security warning so happy to help and get this solved asap.

Copy link
Author

Choose a reason for hiding this comment

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

I'm currently trying to backport that commit. I'll inform you whether I was able to make it.

Copy link
Author

@kretajak kretajak Jun 11, 2025

Choose a reason for hiding this comment

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

I have added new commit which should move us a bit closer. Seems like also changes from 6045b1e might be required as they are tightly connected to 03d1214.

@slorber feel free to continue the work.

headers["sec-fetch-mode"] === "no-cors" &&
headers["sec-fetch-site"] === "cross-site"
) {
res.statusCode = 403;
res.end("Cross-Origin request blocked");
return;
}

next();
},
});

if (typeof this.options.headers !== "undefined") {
middlewares.push({
name: "set-headers",
Expand Down Expand Up @@ -2598,8 +2625,8 @@ class Server {

if (
!headers ||
!this.checkHeader(headers, "host") ||
!this.checkHeader(headers, "origin")
!this.checkHeader(headers, "host", true) ||
!this.checkHeader(headers, "origin", false)
) {
this.sendMessage([client], "error", "Invalid Host/Origin header");

Expand Down Expand Up @@ -3055,9 +3082,10 @@ class Server {
* @private
* @param {{ [key: string]: string | undefined }} headers
* @param {string} headerToCheck
* @param {boolean} allowIP
* @returns {boolean}
*/
checkHeader(headers, headerToCheck) {
checkHeader(headers, headerToCheck, allowIP) {
// allow user to opt out of this security check, at their own risk
// by explicitly enabling allowedHosts
if (this.options.allowedHosts === "all") {
Expand All @@ -3084,7 +3112,10 @@ class Server {
true
).hostname;

// always allow requests with explicit IPv4 or IPv6-address.
// allow requests with explicit IPv4 or IPv6-address if allowIP is true.
// Note that IP should not be automatically allowed for Origin headers,
// otherwise an untrusted remote IP host can send requests.
//
// A note on IPv6 addresses:
// hostHeader will always contain the brackets denoting
// an IPv6-address in URLs,
Expand All @@ -3094,8 +3125,9 @@ class Server {
// and its subdomains (hostname.endsWith(".localhost")).
// allow hostname of listening address (hostname === this.options.host)
const isValidHostname =
(hostname !== null && ipaddr.IPv4.isValid(hostname)) ||
(hostname !== null && ipaddr.IPv6.isValid(hostname)) ||
(allowIP &&
hostname !== null &&
(ipaddr.IPv4.isValid(hostname) || ipaddr.IPv6.isValid(hostname))) ||
hostname === "localhost" ||
(hostname !== null && hostname.endsWith(".localhost")) ||
hostname === this.options.host;
Expand Down
26 changes: 26 additions & 0 deletions test/e2e/__snapshots__/allowed-hosts.test.js.snap.webpack5
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,32 @@ exports[`allowed hosts should disconnect web client using localhost to web socke

exports[`allowed hosts should disconnect web client using localhost to web socket server with the "auto" value ("ws"): page errors 1`] = `Array []`;

exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("sockjs"): console messages 1`] = `
[
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
"[HMR] Waiting for update signal from WDS...",
"Hey.",
"[webpack-dev-server] Invalid Host/Origin header",
"[webpack-dev-server] Disconnected!",
"[webpack-dev-server] Trying to reconnect...",
]
`;

exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("sockjs"): page errors 1`] = `[]`;

exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("ws"): console messages 1`] = `
[
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
"[HMR] Waiting for update signal from WDS...",
"Hey.",
"[webpack-dev-server] Invalid Host/Origin header",
"[webpack-dev-server] Disconnected!",
"[webpack-dev-server] Trying to reconnect...",
]
`;

exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("ws"): page errors 1`] = `[]`;

exports[`allowed hosts should disconnect web socket client using custom hostname from web socket server with the "auto" value based on the "host" header ("sockjs"): console messages 1`] = `
Array [
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
Expand Down
80 changes: 80 additions & 0 deletions test/e2e/allowed-hosts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,86 @@ describe("allowed hosts", () => {
await browser.close();
await server.stop();
});

it(`should disconnect web client with origin header containing an IP address with the "auto" value ("${webSocketServer}")`, async () => {
const devServerHost = "127.0.0.1";
const devServerPort = port1;
const proxyHost = devServerHost;
const proxyPort = port2;

const compiler = webpack(config);
const devServerOptions = {
client: {
webSocketURL: {
port: port2,
},
},
webSocketServer,
port: devServerPort,
host: devServerHost,
allowedHosts: "auto",
};
const server = new Server(devServerOptions, compiler);

await server.start();

function startProxy(callback) {
const app = express();

app.use(
"/",
createProxyMiddleware({
// Emulation
onProxyReqWs: (proxyReq) => {
proxyReq.setHeader("origin", "http://192.168.1.1/");
},
target: `http://${devServerHost}:${devServerPort}`,
ws: true,
changeOrigin: true,
logLevel: "warn",
})
);

return app.listen(proxyPort, proxyHost, callback);
}

const proxy = await new Promise((resolve) => {
const proxyCreated = startProxy(() => {
resolve(proxyCreated);
});
});

const { page, browser } = await runBrowser();

try {
const pageErrors = [];
const consoleMessages = [];

page
.on("console", (message) => {
consoleMessages.push(message);
})
.on("pageerror", (error) => {
pageErrors.push(error);
});

await page.goto(`http://${proxyHost}:${proxyPort}/`, {
waitUntil: "networkidle0",
});

expect(
consoleMessages.map((message) => message.text())
).toMatchSnapshot("console messages");
expect(pageErrors).toMatchSnapshot("page errors");
} catch (error) {
throw error;
} finally {
proxy.close();

await browser.close();
await server.stop();
}
});
}

describe("check host headers", () => {
Expand Down
118 changes: 118 additions & 0 deletions test/e2e/cross-origin-request.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
"use strict";

const webpack = require("webpack");
const Server = require("../../lib/Server");
const config = require("../fixtures/client-config/webpack.config");
const runBrowser = require("../helpers/run-browser");
const [port1, port2] = require("../ports-map")["cross-origin-request"];

describe("cross-origin requests", () => {
const devServerPort = port1;
const htmlServerPort = port2;
const htmlServerHost = "127.0.0.1";

it("should return 403 for cross-origin no-cors non-module script tag requests", async () => {
const compiler = webpack(config);
const devServerOptions = {
port: devServerPort,
allowedHosts: "auto",
};
const server = new Server(devServerOptions, compiler);

await server.start();

// Start a separate server for serving the HTML file
const http = require("http");
const htmlServer = http.createServer((req, res) => {
res.writeHead(200, { "Content-Type": "text/html" });
res.end(`
<html>
<head>
<script src="http://localhost:${devServerPort}/main.js"></script>
</head>
<body></body>
</html>
`);
});
htmlServer.listen(htmlServerPort, htmlServerHost);

const { page, browser } = await runBrowser();
try {
const pageErrors = [];

page.on("pageerror", (error) => {
pageErrors.push(error);
});

const scriptTagRequest = page.waitForResponse(
`http://localhost:${devServerPort}/main.js`
);

await page.goto(`http://${htmlServerHost}:${htmlServerPort}`);

const response = await scriptTagRequest;

expect(response.status()).toBe(403);
} catch (error) {
throw error;
} finally {
await browser.close();
await server.stop();
htmlServer.close();
}
});

it("should return 200 for cross-origin cors non-module script tag requests", async () => {
const compiler = webpack(config);
const devServerOptions = {
port: devServerPort,
allowedHosts: "auto",
headers: {
"Access-Control-Allow-Origin": "*",
},
};
const server = new Server(devServerOptions, compiler);

await server.start();

// Start a separate server for serving the HTML file
const http = require("http");
const htmlServer = http.createServer((req, res) => {
res.writeHead(200, { "Content-Type": "text/html" });
res.end(`
<html>
<head>
<script src="http://localhost:${devServerPort}/main.js" crossorigin></script>
</head>
<body></body>
</html>
`);
});
htmlServer.listen(htmlServerPort, htmlServerHost);

const { page, browser } = await runBrowser();
try {
const pageErrors = [];

page.on("pageerror", (error) => {
pageErrors.push(error);
});

const scriptTagRequest = page.waitForResponse(
`http://localhost:${devServerPort}/main.js`
);

await page.goto(`http://${htmlServerHost}:${htmlServerPort}`);

const response = await scriptTagRequest;

expect(response.status()).toBe(200);
} catch (error) {
throw error;
} finally {
await browser.close();
await server.stop();
htmlServer.close();
}
});
});
1 change: 1 addition & 0 deletions test/ports-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const listOfTests = {
"normalize-option": 1,
"setup-middlewares-option": 1,
"options-request-response": 2,
"cross-origin-request": 2,
};

let startPort = 8089;
Expand Down
1 change: 1 addition & 0 deletions types/lib/Server.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3463,6 +3463,7 @@ declare class Server {
* @private
* @param {{ [key: string]: string | undefined }} headers
* @param {string} headerToCheck
* @param {boolean} allowIP
* @returns {boolean}
*/
private checkHeader;
Expand Down