Skip to content

Commit cb1534a

Browse files
Merge commit from fork
* test: add regression for proxied websocket upgrade origin validation Ref: GHSA-cfqc-fj3v-j925 * fix: validate host and origin on proxied websocket upgrades Upgrade requests bypass the middleware chain, so allowedHosts and the cross-origin checks did not run for a proxied WebSocket. Mirror the host/origin validation the HMR socket performs before forwarding to the proxy, rejecting cross-origin upgrades. Ref: GHSA-cfqc-fj3v-j925 * doc: add changeset * fix: prevent proxy websocket validation bypass after HTTP requests --------- Co-authored-by: Sebastian Beltran <bjohansebas@gmail.com>
1 parent be1ca90 commit cb1534a

5 files changed

Lines changed: 270 additions & 11 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: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ class Server {
415415
// Keep track of websocket proxies for external websocket upgrade.
416416
/**
417417
* @private
418-
* @type {RequestHandler[]}
418+
* @type {(() => RequestHandler | undefined)[]}
419419
*/
420420
this.webSocketProxies = [];
421421
/**
@@ -1755,7 +1755,7 @@ class Server {
17551755
// Proxy WebSocket without the initial http request
17561756
// https://github.com/chimurai/http-proxy-middleware#external-websocket-upgrade
17571757
const webSocketProxies =
1758-
/** @type {RequestHandler[]} */
1758+
/** @type {(() => RequestHandler | undefined)[]} */
17591759
(this.webSocketProxies);
17601760

17611761
const hmrPath =
@@ -1768,13 +1768,15 @@ class Server {
17681768
(this.options.webSocketServer).options
17691769
).path;
17701770

1771-
for (const webSocketProxy of webSocketProxies) {
1772-
const proxyUpgrade =
1773-
/** @type {RequestHandler & { upgrade: NonNullable<RequestHandler["upgrade"]> }} */
1774-
(webSocketProxy).upgrade;
1775-
1771+
for (const getWebSocketProxy of webSocketProxies) {
17761772
/** @type {S} */
17771773
(this.server).on("upgrade", (req, socket, head) => {
1774+
const webSocketProxy = getWebSocketProxy();
1775+
1776+
if (!webSocketProxy) {
1777+
return;
1778+
}
1779+
17781780
if (hmrPath && typeof req.url === "string") {
17791781
// Match the configured HMR path exactly the same way the underlying
17801782
// WebSocket server (`ws`) does in `WebSocketServer#shouldHandle`: a
@@ -1789,7 +1791,31 @@ class Server {
17891791
return;
17901792
}
17911793
}
1792-
proxyUpgrade(req, socket, head);
1794+
// Upgrade requests are dispatched from the raw HTTP server and never
1795+
// enter the middleware chain, so `allowedHosts` and the cross-origin
1796+
// checks do not otherwise run for a proxied WebSocket. Mirror the same
1797+
// host/origin validation the HMR socket performs (see the "connection"
1798+
// handler) so a cross-origin page cannot open a WebSocket through a
1799+
// `ws: true` proxy to a service the developer proxies.
1800+
const headers =
1801+
/** @type {{ [key: string]: string | undefined }} */
1802+
(req.headers);
1803+
1804+
if (
1805+
!headers ||
1806+
!this.isValidHost(headers, "host") ||
1807+
!this.isValidHost(headers, "origin") ||
1808+
!this.isSameOrigin(headers)
1809+
) {
1810+
if (!socket.destroyed) {
1811+
socket.write("HTTP/1.1 403 Forbidden\r\nConnection: close\r\n\r\n");
1812+
socket.destroy();
1813+
}
1814+
1815+
return;
1816+
}
1817+
1818+
webSocketProxy.upgrade(req, socket, head);
17931819
});
17941820
}
17951821
}
@@ -2196,6 +2222,9 @@ class Server {
21962222
return createProxyMiddleware({
21972223
...proxyConfig,
21982224
pathFilter: /** @type {string} */ (context),
2225+
// Handle upgrades ourselves so an HTTP request cannot register an
2226+
// additional listener that bypasses our host/origin and HMR checks.
2227+
ws: false,
21992228
});
22002229
};
22012230

@@ -2231,8 +2260,12 @@ class Server {
22312260
/** @type {RequestHandler} */
22322261
(getProxyMiddleware(proxyConfig));
22332262

2234-
if (proxyConfig.ws) {
2235-
this.webSocketProxies.push(proxyMiddleware);
2263+
if (proxyConfig.ws || typeof proxyConfigOrCallback === "function") {
2264+
// Callbacks can replace the middleware or enable WebSockets after
2265+
// an HTTP request. Resolve the current configuration on each upgrade.
2266+
this.webSocketProxies.push(() =>
2267+
proxyConfig.ws ? proxyMiddleware : undefined,
2268+
);
22362269
}
22372270

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

‎test/ports-map.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ const listOfTests = {
8585
compatibility: 2,
8686
"react-refresh": 1,
8787
"universal-target": 1,
88+
"proxy-web-socket-origin": 2,
8889
};
8990

9091
let startPort = 8089;

‎test/server/proxy-option.test.js‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,9 @@ describe("proxy option", () => {
854854
const setup = async (devServerOptions) => {
855855
backendUpgradeCount = 0;
856856

857-
backend = http.createServer();
857+
backend = http.createServer((req, res) => {
858+
res.end("backend response");
859+
});
858860
backendWss = new WebSocketServer({ server: backend });
859861
backendWss.on("connection", () => {
860862
backendUpgradeCount += 1;
@@ -938,6 +940,7 @@ describe("proxy option", () => {
938940
];
939941

940942
for (const { type, hmrPath, nonHmrPath } of serverTypes) {
943+
// eslint-disable-next-line no-loop-func
941944
describe(`with webSocketServerType: ${type}`, () => {
942945
before(() => setup({ webSocketServer: type }));
943946

@@ -950,6 +953,19 @@ describe("proxy option", () => {
950953
expect(forwarded).toBe(false);
951954
});
952955

956+
it("does not forward the HMR upgrade after an HTTP proxy request", async () => {
957+
await request(server.server).get("/api/hello").expect(200);
958+
959+
const { opened } = await probe(hmrPath);
960+
961+
await new Promise((resolve) => {
962+
setTimeout(resolve, 100);
963+
});
964+
965+
expect(opened).toBe(true);
966+
expect(backendUpgradeCount).toBe(0);
967+
});
968+
953969
it("forwards a non-HMR upgrade to the user proxy", async () => {
954970
const { forwarded } = await probe(nonHmrPath);
955971

0 commit comments

Comments
 (0)