Skip to content

Commit 2a34635

Browse files
committed
Fix security issue
WebSocket messages should not be processed until `initially` has finished and has a chance to optionally close the connection completely. Disconnects and protocol-level errors are processed regardless - these occur at a lower level and should be preserved for consistency with connects.
1 parent cbf31ca commit 2a34635

File tree

1 file changed

+16
-2
lines changed

1 file changed

+16
-2
lines changed

hapi-plugin-websocket.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,10 @@ const register = async (server, pluginOptions) => {
215215
delete headers["accept-encoding"]
216216

217217
/* optionally inject an empty initial message */
218+
let initially;
218219
if (routeOptions.initially) {
219220
/* inject incoming WebSocket message as a simulated HTTP request */
220-
server.inject({
221+
initially = server.inject({
221222
/* simulate the hard-coded POST request */
222223
method: "POST",
223224

@@ -236,14 +237,17 @@ const register = async (server, pluginOptions) => {
236237
}).then(response => {
237238
/* any HTTP redirection, client error or server error response
238239
leads to an immediate WebSocket connection drop */
239-
if (response.statusCode >= 300) {
240+
if (response.statusCode < 300) {
241+
return true;
242+
} else {
240243
const annotation = `(HAPI handler responded with HTTP status ${response.statusCode})`
241244
if (response.statusCode < 400)
242245
ws.close(1002, `Protocol Error ${annotation}`)
243246
else if (response.statusCode < 500)
244247
ws.close(1008, `Policy Violation ${annotation}`)
245248
else
246249
ws.close(1011, `Server Error ${annotation}`)
250+
return false;
247251
}
248252
});
249253
}
@@ -252,6 +256,10 @@ const register = async (server, pluginOptions) => {
252256
if (routeOptions.frame === true) {
253257
/* framed WebSocket communication (correlated request/reply) */
254258
wsf.on("message", async (ev) => {
259+
if (initially && !(await initially)) {
260+
return;
261+
}
262+
255263
/* allow application to hook into raw WebSocket frame processing */
256264
routeOptions.frameMessage.call(ctx, { ctx, wss, ws, wsf, req, peers }, ev.frame)
257265

@@ -294,6 +302,10 @@ const register = async (server, pluginOptions) => {
294302
else {
295303
/* plain WebSocket communication (uncorrelated request/response) */
296304
ws.on("message", async (message) => {
305+
if (initially && !(await initially)) {
306+
return;
307+
}
308+
297309
/* inject incoming WebSocket message as a simulated HTTP request */
298310
const response = await server.inject({
299311
/* simulate the hard-coded POST request */
@@ -322,6 +334,7 @@ const register = async (server, pluginOptions) => {
322334
/* hook into WebSocket disconnection */
323335
ws.on("close", () => {
324336
/* allow application to hook into WebSocket disconnection */
337+
/* note that this is done even if the `initially` handler closes the connection */
325338
routeOptions.disconnect.call(ctx, { ctx, wss, ws, wsf, req, peers })
326339

327340
/* stop tracking the peer */
@@ -330,6 +343,7 @@ const register = async (server, pluginOptions) => {
330343
})
331344

332345
/* allow application to hook into WebSocket error processing */
346+
/* note that this is done even if the `initially` handler closes the connection */
333347
ws.on("error", (error) => {
334348
routeOptions.error.call(ctx, { ctx, wss, ws, wsf, req, peers }, error)
335349
})

0 commit comments

Comments
 (0)