socketio · Apr 19, 2023 · May 1, 2023 · May 1, 2023 · May 1, 2023 · May 1, 2023
Showing with 290 additions and 111 deletions.

+34 −0 CHANGELOG.md

+42 −29 lib/server.ts

+2 −4 lib/transports/polling.ts

+94 −77 lib/userver.ts

+1 −1 package.json

+71 −0 test/middlewares.js

+46 −0 test/server.js
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## 2023
 
+- [6.4.2](#642-2023-05-02) (May 2023)
 - [6.4.1](#641-2023-02-20) (Feb 2023)
 - [6.4.0](#640-2023-02-06) (Feb 2023)
 - [6.3.1](#631-2023-01-12) (Jan 2023)
@@ -46,6 +47,39 @@
 
 # Release notes
 
+## [6.4.2](https://github.com/socketio/engine.io/compare/6.4.1...6.4.2) (2023-05-02)
+
+:warning: This release contains an important security fix :warning:
+
+A malicious client could send a specially crafted HTTP request, triggering an uncaught exception and killing the Node.js process:
+
+```
+TypeError: Cannot read properties of undefined (reading 'handlesUpgrades')
+  at Server.onWebSocket (build/server.js:515:67)
+```
+
+Please upgrade as soon as possible.
+
+
+### Bug Fixes
+
+* include error handling for Express middlewares ([#674](https://github.com/socketio/engine.io/issues/674)) ([9395782](https://github.com/socketio/engine.io/commit/93957828be1252c83275b56f0c7c0bd145a0ceb9))
+* prevent crash when provided with an invalid query param ([fc480b4](https://github.com/socketio/engine.io/commit/fc480b4f305e16fe5972cf337d055e598372dc44))
+* **typings:** make clientsCount public ([#675](https://github.com/socketio/engine.io/issues/675)) ([bd6d471](https://github.com/socketio/engine.io/commit/bd6d4713b02ff646c581872cd9ffe753acff0d73))
+* **uws:** prevent crash when using with middlewares ([8b22162](https://github.com/socketio/engine.io/commit/8b2216290330b174c9e67be32765bec0c74769f9))
+
+
+### Credits
+
+Huge thanks to [@tyilo](https://github.com/tyilo) and [@cieldeville](https://github.com/cieldeville) for helping!
+
+
+### Dependencies
+
+- [`ws@~8.11.0`](https://github.com/websockets/ws/releases/tag/8.11.0) (no change)
+
+
+
 ## [6.4.1](https://github.com/socketio/engine.io/compare/6.4.0...6.4.1) (2023-02-20)
 
 This release contains [6e78489](https://github.com/socketio/engine.io/commit/6e78489486f0d7570861fd6002a364d1ab87da4a), which exports the `BaseServer` class in order to restore the compatibility with the `nodenext` module resolution strategy of TypeScript.

diff --git a/lib/server.ts b/lib/server.ts
@@ -137,14 +137,14 @@ export interface ServerOptions {
 type Middleware = (
   req: IncomingMessage,
   res: ServerResponse,
-  next: () => void
+  next: (err?: any) => void
 ) => void;
 
 export abstract class BaseServer extends EventEmitter {
   public opts: ServerOptions;
 
   protected clients: any;
-  private clientsCount: number;
+  public clientsCount: number;
   protected middlewares: Middleware[] = [];
 
   /**
@@ -320,7 +320,7 @@ export abstract class BaseServer extends EventEmitter {
    *
    * @param fn
    */
-  public use(fn: Middleware) {
+  public use(fn: any) {
     this.middlewares.push(fn);
   }
 
@@ -335,7 +335,7 @@ export abstract class BaseServer extends EventEmitter {
   protected _applyMiddlewares(
     req: IncomingMessage,
     res: ServerResponse,
-    callback: () => void
+    callback: (err?: any) => void
   ) {
     if (this.middlewares.length === 0) {
       debug("no middleware to apply, skipping");
@@ -344,7 +344,11 @@ export abstract class BaseServer extends EventEmitter {
 
     const apply = (i) => {
       debug("applying middleware n°%d", i + 1);
-      this.middlewares[i](req, res, () => {
+      this.middlewares[i](req, res, (err?: any) => {
+        if (err) {
+          return callback(err);
+        }
+
         if (i + 1 < this.middlewares.length) {
           apply(i + 1);
         } else {
@@ -655,8 +659,12 @@ export class Server extends BaseServer {
       }
     };
 
-    this._applyMiddlewares(req, res, () => {
-      this.verify(req, false, callback);
+    this._applyMiddlewares(req, res, (err) => {
+      if (err) {
+        callback(Server.errors.BAD_REQUEST, { name: "MIDDLEWARE_FAILURE" });
+      } else {
+        this.verify(req, false, callback);
+      }
     });
   }
 
@@ -673,32 +681,37 @@ export class Server extends BaseServer {
     this.prepare(req);
 
     const res = new WebSocketResponse(req, socket);
+    const callback = (errorCode, errorContext) => {
+      if (errorCode !== undefined) {
+        this.emit("connection_error", {
+          req,
+          code: errorCode,
+          message: Server.errorMessages[errorCode],
+          context: errorContext,
+        });
+        abortUpgrade(socket, errorCode, errorContext);
+        return;
+      }
 
-    this._applyMiddlewares(req, res as unknown as ServerResponse, () => {
-      this.verify(req, true, (errorCode, errorContext) => {
-        if (errorCode) {
-          this.emit("connection_error", {
-            req,
-            code: errorCode,
-            message: Server.errorMessages[errorCode],
-            context: errorContext,
-          });
-          abortUpgrade(socket, errorCode, errorContext);
-          return;
-        }
-
-        const head = Buffer.from(upgradeHead);
-        upgradeHead = null;
+      const head = Buffer.from(upgradeHead);
+      upgradeHead = null;
 
-        // some middlewares (like express-session) wait for the writeHead() call to flush their headers
-        // see https://github.com/expressjs/session/blob/1010fadc2f071ddf2add94235d72224cf65159c6/index.js#L220-L244
-        res.writeHead();
+      // some middlewares (like express-session) wait for the writeHead() call to flush their headers
+      // see https://github.com/expressjs/session/blob/1010fadc2f071ddf2add94235d72224cf65159c6/index.js#L220-L244
+      res.writeHead();
 
-        // delegate to ws
-        this.ws.handleUpgrade(req, socket, head, (websocket) => {
-          this.onWebSocket(req, socket, websocket);
-        });
+      // delegate to ws
+      this.ws.handleUpgrade(req, socket, head, (websocket) => {
+        this.onWebSocket(req, socket, websocket);
       });
+    };
+
+    this._applyMiddlewares(req, res as unknown as ServerResponse, (err) => {
+      if (err) {
+        callback(Server.errors.BAD_REQUEST, { name: "MIDDLEWARE_FAILURE" });
+      } else {
+        this.verify(req, true, callback);
+      }
     });
   }
 

diff --git a/lib/transports/polling.ts b/lib/transports/polling.ts
@@ -75,8 +75,7 @@ export class Polling extends Transport {
       debug("request overlap");
       // assert: this.res, '.req and .res should be (un)set together'
       this.onError("overlap from client");
-      // TODO for the next major release: use an HTTP 400 status code (https://github.com/socketio/engine.io/issues/650)
-      res.writeHead(500);
+      res.writeHead(400);
       res.end();
       return;
     }
@@ -117,8 +116,7 @@ export class Polling extends Transport {
     if (this.dataReq) {
       // assert: this.dataRes, '.dataReq and .dataRes should be (un)set together'
       this.onError("data request overlap from client");
-      // TODO for the next major release: use an HTTP 400 status code (https://github.com/socketio/engine.io/issues/650)
-      res.writeHead(500);
+      res.writeHead(400);
       res.end();
       return;
     }