Ylianst / MeshCentral

A complete web-based remote monitoring and management web site. Once setup you can install agents and perform remote desktop session to devices on the local network or over the Internet.
https://meshcentral.com
Apache License 2.0
4.28k stars 573 forks source link

Added global ws error handler #6475

Closed HuFlungDu closed 4 weeks ago

HuFlungDu commented 1 month ago

In some cases, a client could send packets that would cause the webserver to error and crash. In my case, I was getting the error:

RangeError: Invalid WebSocket frame: MASK must be set
     at Receiver.getInfo (meshcentral/node_modules/express-ws/node_modules/ws/lib/receiver.js:284:16)
     at Receiver.startLoop (meshcentral/node_modules/express-ws/node_modules/ws/lib/receiver.js:131:22)
     at Receiver._write (meshcentral/node_modules/express-ws/node_modules/ws/lib/receiver.js:78:10)
     at writeOrBuffer (node:internal/streams/writable:570:12)
     at _write (node:internal/streams/writable:499:10)
     at Writable.write (node:internal/streams/writable:508:10)
     at TLSSocket.socketOnData (meshcentral/node_modules/express-ws/node_modules/ws/lib/websocket.js:1164:35)
     at TLSSocket.emit (node:events:519:28)
     at addChunk (node:internal/streams/readable:559:12)
     at readableAddChunkPushByteMode (node:internal/streams/readable:510:3) {
   code: 'WS_ERR_EXPECTED_MASK',
   [Symbol(status-code)]: 1002
}

Whatever WS handler this is propagating to is not handling the "error" event on the ws object. I couldn't track it down since this only happens in production for me, so I can't replicate it in a safe environment for testing.

While this is error is definitely worth tracking down itself (#5433?), the server should also never crash due to bad client input. This creates a failsafe in the case there is an unhandled error

si458 commented 4 weeks ago

thanks for this, amazingly, we already have a catch in place for the meshagent ws but not web clients https://github.com/Ylianst/MeshCentral/blob/141bec559fb291a0f361826d3000851a118e08cb/meshagent.js#L577

linked issue here showing the same thing but for meshagents https://github.com/Ylianst/MeshCentral/issues/6463