Skip to content

Commit 5ba835f

Browse files
sapphi-redkretajak
authored andcommitted
fix(security): requests with an IP addresses Origin header are not allowed to connect to WebSocket
1 parent 11bfcde commit 5ba835f

File tree

4 files changed

+120
-7
lines changed

4 files changed

+120
-7
lines changed

lib/Server.js

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2014,7 +2014,8 @@ class Server {
20142014
this.checkHeader(
20152015
/** @type {{ [key: string]: string | undefined }} */
20162016
(req.headers),
2017-
"host"
2017+
"host",
2018+
true
20182019
)
20192020
) {
20202021
return next();
@@ -2598,8 +2599,8 @@ class Server {
25982599

25992600
if (
26002601
!headers ||
2601-
!this.checkHeader(headers, "host") ||
2602-
!this.checkHeader(headers, "origin")
2602+
!this.checkHeader(headers, "host", true) ||
2603+
!this.checkHeader(headers, "origin", false)
26032604
) {
26042605
this.sendMessage([client], "error", "Invalid Host/Origin header");
26052606

@@ -3055,9 +3056,10 @@ class Server {
30553056
* @private
30563057
* @param {{ [key: string]: string | undefined }} headers
30573058
* @param {string} headerToCheck
3059+
* @param {boolean} allowIP
30583060
* @returns {boolean}
30593061
*/
3060-
checkHeader(headers, headerToCheck) {
3062+
checkHeader(headers, headerToCheck, allowIP) {
30613063
// allow user to opt out of this security check, at their own risk
30623064
// by explicitly enabling allowedHosts
30633065
if (this.options.allowedHosts === "all") {
@@ -3084,7 +3086,10 @@ class Server {
30843086
true
30853087
).hostname;
30863088

3087-
// always allow requests with explicit IPv4 or IPv6-address.
3089+
// allow requests with explicit IPv4 or IPv6-address if allowIP is true.
3090+
// Note that IP should not be automatically allowed for Origin headers,
3091+
// otherwise an untrusted remote IP host can send requests.
3092+
//
30883093
// A note on IPv6 addresses:
30893094
// hostHeader will always contain the brackets denoting
30903095
// an IPv6-address in URLs,
@@ -3094,8 +3099,9 @@ class Server {
30943099
// and its subdomains (hostname.endsWith(".localhost")).
30953100
// allow hostname of listening address (hostname === this.options.host)
30963101
const isValidHostname =
3097-
(hostname !== null && ipaddr.IPv4.isValid(hostname)) ||
3098-
(hostname !== null && ipaddr.IPv6.isValid(hostname)) ||
3102+
(allowIP &&
3103+
hostname !== null &&
3104+
(ipaddr.IPv4.isValid(hostname) || ipaddr.IPv6.isValid(hostname))) ||
30993105
hostname === "localhost" ||
31003106
(hostname !== null && hostname.endsWith(".localhost")) ||
31013107
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
@@ -274,6 +274,32 @@ exports[`allowed hosts should disconnect web client using localhost to web socke
274274

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

277+
exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("sockjs"): console messages 1`] = `
278+
[
279+
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
280+
"[HMR] Waiting for update signal from WDS...",
281+
"Hey.",
282+
"[webpack-dev-server] Invalid Host/Origin header",
283+
"[webpack-dev-server] Disconnected!",
284+
"[webpack-dev-server] Trying to reconnect...",
285+
]
286+
`;
287+
288+
exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("sockjs"): page errors 1`] = `[]`;
289+
290+
exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("ws"): console messages 1`] = `
291+
[
292+
"[webpack-dev-server] Server started: Hot Module Replacement enabled, Live Reloading enabled, Progress disabled, Overlay enabled.",
293+
"[HMR] Waiting for update signal from WDS...",
294+
"Hey.",
295+
"[webpack-dev-server] Invalid Host/Origin header",
296+
"[webpack-dev-server] Disconnected!",
297+
"[webpack-dev-server] Trying to reconnect...",
298+
]
299+
`;
300+
301+
exports[`allowed hosts should disconnect web client with origin header containing an IP address with the "auto" value ("ws"): page errors 1`] = `[]`;
302+
277303
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`] = `
278304
Array [
279305
"[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
@@ -1147,6 +1147,86 @@ describe("allowed hosts", () => {
11471147
await browser.close();
11481148
await server.stop();
11491149
});
1150+
1151+
it(`should disconnect web client with origin header containing an IP address with the "auto" value ("${webSocketServer}")`, async () => {
1152+
const devServerHost = "127.0.0.1";
1153+
const devServerPort = port1;
1154+
const proxyHost = devServerHost;
1155+
const proxyPort = port2;
1156+
1157+
const compiler = webpack(config);
1158+
const devServerOptions = {
1159+
client: {
1160+
webSocketURL: {
1161+
port: port2,
1162+
},
1163+
},
1164+
webSocketServer,
1165+
port: devServerPort,
1166+
host: devServerHost,
1167+
allowedHosts: "auto",
1168+
};
1169+
const server = new Server(devServerOptions, compiler);
1170+
1171+
await server.start();
1172+
1173+
function startProxy(callback) {
1174+
const app = express();
1175+
1176+
app.use(
1177+
"/",
1178+
createProxyMiddleware({
1179+
// Emulation
1180+
onProxyReqWs: (proxyReq) => {
1181+
proxyReq.setHeader("origin", "http://192.168.1.1/");
1182+
},
1183+
target: `http://${devServerHost}:${devServerPort}`,
1184+
ws: true,
1185+
changeOrigin: true,
1186+
logLevel: "warn",
1187+
})
1188+
);
1189+
1190+
return app.listen(proxyPort, proxyHost, callback);
1191+
}
1192+
1193+
const proxy = await new Promise((resolve) => {
1194+
const proxyCreated = startProxy(() => {
1195+
resolve(proxyCreated);
1196+
});
1197+
});
1198+
1199+
const { page, browser } = await runBrowser();
1200+
1201+
try {
1202+
const pageErrors = [];
1203+
const consoleMessages = [];
1204+
1205+
page
1206+
.on("console", (message) => {
1207+
consoleMessages.push(message);
1208+
})
1209+
.on("pageerror", (error) => {
1210+
pageErrors.push(error);
1211+
});
1212+
1213+
await page.goto(`http://${proxyHost}:${proxyPort}/`, {
1214+
waitUntil: "networkidle0",
1215+
});
1216+
1217+
expect(
1218+
consoleMessages.map((message) => message.text())
1219+
).toMatchSnapshot("console messages");
1220+
expect(pageErrors).toMatchSnapshot("page errors");
1221+
} catch (error) {
1222+
throw error;
1223+
} finally {
1224+
proxy.close();
1225+
1226+
await browser.close();
1227+
await server.stop();
1228+
}
1229+
});
11501230
}
11511231

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

types/lib/Server.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3463,6 +3463,7 @@ declare class Server {
34633463
* @private
34643464
* @param {{ [key: string]: string | undefined }} headers
34653465
* @param {string} headerToCheck
3466+
* @param {boolean} allowIP
34663467
* @returns {boolean}
34673468
*/
34683469
private checkHeader;

0 commit comments

Comments
 (0)