Skip to content

Commit 3e9b604

Browse files
authored
Merge commit from fork
1 parent 8a37b0e commit 3e9b604

5 files changed

Lines changed: 309 additions & 13 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack-dev-server": patch
3+
---
4+
5+
Validate the `Host` and `Origin` headers and enforce same-origin checks on WebSocket upgrade requests handled by proxies with `ws: true`. This prevents cross-origin pages from connecting to proxied WebSocket services through the development server.

‎lib/Server.js‎

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ class Server {
354354
// Keep track of websocket proxies for external websocket upgrade.
355355
/**
356356
* @private
357-
* @type {RequestHandler[]}
357+
* @type {(() => RequestHandler | undefined)[]}
358358
*/
359359
this.webSocketProxies = [];
360360
/**
@@ -1849,7 +1849,7 @@ class Server {
18491849
// Proxy WebSocket without the initial http request
18501850
// https://github.com/chimurai/http-proxy-middleware#external-websocket-upgrade
18511851
const webSocketProxies =
1852-
/** @type {RequestHandler[]} */
1852+
/** @type {(() => RequestHandler | undefined)[]} */
18531853
(this.webSocketProxies);
18541854

18551855
const hmrPath =
@@ -1862,13 +1862,15 @@ class Server {
18621862
(this.options.webSocketServer).options
18631863
).path;
18641864

1865-
for (const webSocketProxy of webSocketProxies) {
1866-
const proxyUpgrade =
1867-
/** @type {RequestHandler & { upgrade: NonNullable<RequestHandler["upgrade"]> }} */
1868-
(webSocketProxy).upgrade;
1869-
1865+
for (const getWebSocketProxy of webSocketProxies) {
18701866
/** @type {S} */
18711867
(this.server).on("upgrade", (req, socket, head) => {
1868+
const webSocketProxy = getWebSocketProxy();
1869+
1870+
if (!webSocketProxy) {
1871+
return;
1872+
}
1873+
18721874
if (hmrPath && typeof req.url === "string") {
18731875
// Match the configured HMR path exactly the same way the underlying
18741876
// WebSocket server (`ws`) does in `WebSocketServer#shouldHandle`: a
@@ -1883,6 +1885,30 @@ class Server {
18831885
return;
18841886
}
18851887
}
1888+
// Upgrade requests bypass the middleware chain. Apply the same
1889+
// host/origin validation as the HMR socket before reaching the proxy.
1890+
const headers =
1891+
/** @type {{ [key: string]: string | undefined }} */
1892+
(req.headers);
1893+
1894+
if (
1895+
!headers ||
1896+
!this.isValidHost(headers, "host") ||
1897+
!this.isValidHost(headers, "origin") ||
1898+
!this.isSameOrigin(headers)
1899+
) {
1900+
if (!socket.destroyed) {
1901+
socket.write("HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n");
1902+
socket.destroy();
1903+
}
1904+
1905+
return;
1906+
}
1907+
1908+
const proxyUpgrade =
1909+
/** @type {RequestHandler & { upgrade: NonNullable<RequestHandler["upgrade"]> }} */
1910+
(webSocketProxy).upgrade;
1911+
18861912
proxyUpgrade(req, socket, head);
18871913
});
18881914
}
@@ -2309,19 +2335,23 @@ class Server {
23092335
* @returns {RequestHandler | undefined} request handler
23102336
*/
23112337
const getProxyMiddleware = (proxyConfig) => {
2338+
// Handle upgrades ourselves so an HTTP request cannot register an
2339+
// additional listener that bypasses our host/origin and HMR checks.
2340+
const proxyOptions = { ...proxyConfig, ws: false };
2341+
23122342
// It is possible to use the `bypass` method without a `target` or `router`.
23132343
// However, the proxy middleware has no use in this case, and will fail to instantiate.
23142344
if (proxyConfig.target) {
23152345
const context = proxyConfig.context || proxyConfig.path;
23162346

23172347
return createProxyMiddleware(
23182348
/** @type {string} */ (context),
2319-
proxyConfig,
2349+
proxyOptions,
23202350
);
23212351
}
23222352

23232353
if (proxyConfig.router) {
2324-
return createProxyMiddleware(proxyConfig);
2354+
return createProxyMiddleware(proxyOptions);
23252355
}
23262356

23272357
// TODO improve me after drop `bypass` to always generate error when configuration is bad
@@ -2366,8 +2396,12 @@ class Server {
23662396
/** @type {RequestHandler} */
23672397
(getProxyMiddleware(proxyConfig));
23682398

2369-
if (proxyConfig.ws) {
2370-
this.webSocketProxies.push(proxyMiddleware);
2399+
if (proxyConfig.ws || typeof proxyConfigOrCallback === "function") {
2400+
// Callbacks can replace the middleware or enable WebSockets after
2401+
// an HTTP request. Resolve the current configuration on each upgrade.
2402+
this.webSocketProxies.push(() =>
2403+
proxyConfig.ws ? proxyMiddleware : undefined,
2404+
);
23712405
}
23722406

23732407
/**
Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
"use strict";
2+
3+
const fs = require("node:fs");
4+
const http = require("node:http");
5+
const os = require("node:os");
6+
const path = require("node:path");
7+
const { setTimeout } = require("node:timers/promises");
8+
const request = require("supertest");
9+
const webpack = require("webpack");
10+
const { WebSocketServer } = require("ws");
11+
const Server = require("../../lib/Server");
12+
const portsMap = require("../ports-map");
13+
14+
const [devServerPort, backendPort] = portsMap["proxy-web-socket-origin"];
15+
16+
// @see https://github.com/webpack/webpack-dev-server/security/advisories/GHSA-cfqc-fj3v-j925
17+
describe.each(["target", "router"])(
18+
"proxied WebSocket upgrade origin validation with %s",
19+
(proxyOption) => {
20+
let server;
21+
let backend;
22+
let webSocketServer;
23+
let backendConnections;
24+
let tmp;
25+
26+
function createProxyConfig(config) {
27+
if (proxyOption === "router") {
28+
const { target, ...options } = config;
29+
30+
return { ...options, router: () => target };
31+
}
32+
33+
return config;
34+
}
35+
36+
async function startServer(proxy) {
37+
const compiler = webpack({
38+
mode: "development",
39+
context: tmp,
40+
entry: "./entry.js",
41+
output: { path: path.join(tmp, "dist") },
42+
});
43+
44+
server = new Server(
45+
{
46+
port: devServerPort,
47+
host: "localhost",
48+
allowedHosts: "auto",
49+
client: false,
50+
hot: false,
51+
liveReload: false,
52+
proxy: [
53+
typeof proxy === "function"
54+
? () => createProxyConfig(proxy())
55+
: createProxyConfig(proxy),
56+
],
57+
},
58+
compiler,
59+
);
60+
61+
await server.start();
62+
}
63+
64+
beforeEach(async () => {
65+
backendConnections = 0;
66+
67+
// A stand-in for a service the developer proxies over WebSocket.
68+
backend = http.createServer((req, res) => {
69+
res.end("backend response");
70+
});
71+
webSocketServer = new WebSocketServer({ server: backend });
72+
webSocketServer.on("connection", (client) => {
73+
backendConnections += 1;
74+
client.send("INTERNAL-SERVICE-SECRET");
75+
});
76+
await new Promise((resolve) => {
77+
backend.listen(backendPort, "127.0.0.1", resolve);
78+
});
79+
80+
tmp = fs.mkdtempSync(path.join(os.tmpdir(), "wds-proxy-origin-"));
81+
fs.writeFileSync(path.join(tmp, "entry.js"), "console.log('e2e');\n");
82+
83+
await startServer({
84+
context: ["/api"],
85+
target: `http://127.0.0.1:${backendPort}`,
86+
ws: true,
87+
});
88+
});
89+
90+
afterEach(async () => {
91+
await server.stop();
92+
for (const client of webSocketServer.clients) {
93+
client.terminate();
94+
}
95+
webSocketServer.close();
96+
backend.closeAllConnections?.();
97+
await new Promise((resolve) => {
98+
backend.close(resolve);
99+
});
100+
fs.rmSync(tmp, { recursive: true, force: true });
101+
});
102+
103+
function upgrade(origin, headers = {}) {
104+
return new Promise((resolve) => {
105+
const req = http.request({
106+
host: "localhost",
107+
port: devServerPort,
108+
path: "/api/socket",
109+
timeout: 8000,
110+
headers: {
111+
Connection: "Upgrade",
112+
Upgrade: "websocket",
113+
"Sec-WebSocket-Key": "dGhlIHNhbXBsZSBub25jZQ==",
114+
"Sec-WebSocket-Version": "13",
115+
...(typeof origin === "string" ? { Origin: origin } : {}),
116+
...headers,
117+
},
118+
});
119+
120+
req.on("upgrade", (res, socket) => {
121+
socket.destroy();
122+
resolve({ upgraded: true, status: res.statusCode });
123+
});
124+
req.on("response", (res) => {
125+
res.resume();
126+
resolve({ upgraded: false, status: res.statusCode });
127+
});
128+
req.on("timeout", () => {
129+
req.destroy();
130+
resolve({ upgraded: false });
131+
});
132+
req.on("error", () => {
133+
resolve({ upgraded: false });
134+
});
135+
req.end();
136+
});
137+
}
138+
139+
it("should reject a cross-origin proxied WebSocket upgrade", async () => {
140+
const result = await upgrade("http://evil.example.com");
141+
142+
expect(result.upgraded).toBe(false);
143+
expect(result.status).toBe(403);
144+
expect(backendConnections).toBe(0);
145+
});
146+
147+
it("should allow a same-origin proxied WebSocket upgrade", async () => {
148+
const result = await upgrade(`http://localhost:${devServerPort}`);
149+
150+
expect(result.upgraded).toBe(true);
151+
expect(result.status).toBe(101);
152+
expect(backendConnections).toBe(1);
153+
});
154+
155+
it("should reject an upgrade with an invalid Host header", async () => {
156+
const result = await upgrade(`http://localhost:${devServerPort}`, {
157+
Host: "evil.example.com",
158+
});
159+
160+
expect(result.status).toBe(403);
161+
expect(backendConnections).toBe(0);
162+
});
163+
164+
it("should reject an upgrade without an Origin header", async () => {
165+
const result = await upgrade();
166+
167+
expect(result.status).toBe(403);
168+
expect(backendConnections).toBe(0);
169+
});
170+
171+
it("should not forward a rejected upgrade after an HTTP proxy request", async () => {
172+
await request(server.server).get("/api/hello").expect(200);
173+
174+
const result = await upgrade("http://evil.example.com");
175+
176+
expect(result.upgraded).toBe(false);
177+
expect(result.status).toBe(403);
178+
179+
// A separate upgrade listener can still contact the backend after the
180+
// client receives the rejection, so allow that forwarding to finish.
181+
await setTimeout(100);
182+
183+
expect(backendConnections).toBe(0);
184+
});
185+
186+
it("should allow a same-origin upgrade after an HTTP proxy request", async () => {
187+
await request(server.server).get("/api/hello").expect(200);
188+
189+
const result = await upgrade(`http://localhost:${devServerPort}`);
190+
191+
expect(result.upgraded).toBe(true);
192+
expect(result.status).toBe(101);
193+
expect(backendConnections).toBe(1);
194+
});
195+
196+
it("should validate upgrades using the current proxy callback configuration", async () => {
197+
await server.stop();
198+
199+
let proxyConfig = {
200+
context: ["/api"],
201+
target: `http://127.0.0.1:${backendPort}`,
202+
ws: false,
203+
pathRewrite: { "^/api": "/initial" },
204+
};
205+
206+
await startServer(() => ({ ...proxyConfig }));
207+
await request(server.server).get("/api/hello").expect(200);
208+
209+
proxyConfig = {
210+
...proxyConfig,
211+
ws: true,
212+
pathRewrite: { "^/api": "/updated" },
213+
};
214+
215+
await request(server.server).get("/api/hello").expect(200);
216+
217+
const rejected = await upgrade("http://evil.example.com");
218+
219+
expect(rejected.status).toBe(403);
220+
221+
await setTimeout(100);
222+
223+
expect(backendConnections).toBe(0);
224+
225+
let backendPath;
226+
227+
backend.once("upgrade", (req) => {
228+
backendPath = req.url;
229+
});
230+
231+
const allowed = await upgrade(`http://localhost:${devServerPort}`);
232+
233+
expect(allowed.status).toBe(101);
234+
expect(backendConnections).toBe(1);
235+
expect(backendPath).toBe("/updated/socket");
236+
});
237+
},
238+
);

‎test/ports-map.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ const listOfTests = {
8282
"options-request-response": 2,
8383
app: 1,
8484
"cross-origin-request": 2,
85+
"proxy-web-socket-origin": 2,
8586
};
8687

8788
let startPort = 8089;

0 commit comments

Comments
 (0)