Skip to content

Commit d2575ad

Browse files
authored
Merge commit from fork
BREAKING CHANGE: requests with an IP addresses Origin header are not allowed to connect to WebSocket server unless configured by `allowedHosts`
1 parent 8c1abc9 commit d2575ad

File tree

4 files changed

+119
-7
lines changed

4 files changed

+119
-7
lines changed

lib/Server.js

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1960,7 +1960,7 @@ class Server {
19601960
(req.headers);
19611961
const headerName = headers[":authority"] ? ":authority" : "host";
19621962

1963-
if (this.checkHeader(headers, headerName)) {
1963+
if (this.checkHeader(headers, headerName, true)) {
19641964
next();
19651965
return;
19661966
}
@@ -2641,8 +2641,8 @@ class Server {
26412641

26422642
if (
26432643
!headers ||
2644-
!this.checkHeader(headers, "host") ||
2645-
!this.checkHeader(headers, "origin")
2644+
!this.checkHeader(headers, "host", true) ||
2645+
!this.checkHeader(headers, "origin", false)
26462646
) {
26472647
this.sendMessage([client], "error", "Invalid Host/Origin header");
26482648

@@ -3081,9 +3081,10 @@ class Server {
30813081
* @private
30823082
* @param {{ [key: string]: string | undefined }} headers
30833083
* @param {string} headerToCheck
3084+
* @param {boolean} allowIP
30843085
* @returns {boolean}
30853086
*/
3086-
checkHeader(headers, headerToCheck) {
3087+
checkHeader(headers, headerToCheck, allowIP) {
30873088
// allow user to opt out of this security check, at their own risk
30883089
// by explicitly enabling allowedHosts
30893090
if (this.options.allowedHosts === "all") {
@@ -3110,7 +3111,10 @@ class Server {
31103111
true,
31113112
).hostname;
31123113

3113-
// always allow requests with explicit IPv4 or IPv6-address.
3114+
// allow requests with explicit IPv4 or IPv6-address if allowIP is true.
3115+
// Note that IP should not be automatically allowed for Origin headers,
3116+
// otherwise an untrusted remote IP host can send requests.
3117+
//
31143118
// A note on IPv6 addresses:
31153119
// hostHeader will always contain the brackets denoting
31163120
// an IPv6-address in URLs,
@@ -3120,8 +3124,9 @@ class Server {
31203124
// and its subdomains (hostname.endsWith(".localhost")).
31213125
// allow hostname of listening address (hostname === this.options.host)
31223126
const isValidHostname =
3123-
(hostname !== null && ipaddr.IPv4.isValid(hostname)) ||
3124-
(hostname !== null && ipaddr.IPv6.isValid(hostname)) ||
3127+
(allowIP &&
3128+
hostname !== null &&
3129+
(ipaddr.IPv4.isValid(hostname) || ipaddr.IPv6.isValid(hostname))) ||
31253130
hostname === "localhost" ||
31263131
(hostname !== null && hostname.endsWith(".localhost")) ||
31273132
hostname === this.options.host;

test/e2e/__snapshots__/allowed-hosts.test.js.snap.webpack5

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,32 @@ exports[`allowed hosts should disconnect web client using localhost to web socke
282282

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

285+
exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("sockjs"): console messages 1`] = `
286+
[
287+
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
288+
"[HMR] Waiting for update signal from WDS...",
289+
"Hey.",
290+
"[webpack-dev-server] Invalid Host/Origin header",
291+
"[webpack-dev-server] Disconnected!",
292+
"[webpack-dev-server] Trying to reconnect...",
293+
]
294+
`;
295+
296+
exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("sockjs"): page errors 1`] = `[]`;
297+
298+
exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("ws"): console messages 1`] = `
299+
[
300+
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
301+
"[HMR] Waiting for update signal from WDS...",
302+
"Hey.",
303+
"[webpack-dev-server] Invalid Host/Origin header",
304+
"[webpack-dev-server] Disconnected!",
305+
"[webpack-dev-server] Trying to reconnect...",
306+
]
307+
`;
308+
309+
exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("ws"): page errors 1`] = `[]`;
310+
285311
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`] = `
286312
[
287313
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",

test/e2e/allowed-hosts.test.js

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,6 +1206,86 @@ describe("allowed hosts", () => {
12061206
await server.stop();
12071207
}
12081208
});
1209+
1210+
it(`should disconnect web client with origin header containing an IP address with the "auto" value ("${webSocketServer}")`, async () => {
1211+
const devServerHost = "127.0.0.1";
1212+
const devServerPort = port1;
1213+
const proxyHost = devServerHost;
1214+
const proxyPort = port2;
1215+
1216+
const compiler = webpack(config);
1217+
const devServerOptions = {
1218+
client: {
1219+
webSocketURL: {
1220+
port: port2,
1221+
},
1222+
},
1223+
webSocketServer,
1224+
port: devServerPort,
1225+
host: devServerHost,
1226+
allowedHosts: "auto",
1227+
};
1228+
const server = new Server(devServerOptions, compiler);
1229+
1230+
await server.start();
1231+
1232+
function startProxy(callback) {
1233+
const app = express();
1234+
1235+
app.use(
1236+
"/",
1237+
createProxyMiddleware({
1238+
// Emulation
1239+
onProxyReqWs: (proxyReq) => {
1240+
proxyReq.setHeader("origin", "http://192.168.1.1/");
1241+
},
1242+
target: `http://${devServerHost}:${devServerPort}`,
1243+
ws: true,
1244+
changeOrigin: true,
1245+
logLevel: "warn",
1246+
}),
1247+
);
1248+
1249+
return app.listen(proxyPort, proxyHost, callback);
1250+
}
1251+
1252+
const proxy = await new Promise((resolve) => {
1253+
const proxyCreated = startProxy(() => {
1254+
resolve(proxyCreated);
1255+
});
1256+
});
1257+
1258+
const { page, browser } = await runBrowser();
1259+
1260+
try {
1261+
const pageErrors = [];
1262+
const consoleMessages = [];
1263+
1264+
page
1265+
.on("console", (message) => {
1266+
consoleMessages.push(message);
1267+
})
1268+
.on("pageerror", (error) => {
1269+
pageErrors.push(error);
1270+
});
1271+
1272+
await page.goto(`http://${proxyHost}:${proxyPort}/`, {
1273+
waitUntil: "networkidle0",
1274+
});
1275+
1276+
expect(
1277+
consoleMessages.map((message) => message.text()),
1278+
).toMatchSnapshot("console messages");
1279+
expect(pageErrors).toMatchSnapshot("page errors");
1280+
} catch (error) {
1281+
throw error;
1282+
} finally {
1283+
proxy.close();
1284+
1285+
await browser.close();
1286+
await server.stop();
1287+
}
1288+
});
12091289
}
12101290

12111291
describe("check host headers", () => {

types/lib/Server.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,6 +1351,7 @@ declare class Server<
13511351
* @private
13521352
* @param {{ [key: string]: string | undefined }} headers
13531353
* @param {string} headerToCheck
1354+
* @param {boolean} allowIP
13541355
* @returns {boolean}
13551356
*/
13561357
private checkHeader;

0 commit comments

Comments
 (0)