Fix unhandled JSON.parse exception on WebSocket message crashes the server - #155
Open
iaohkut-from-NightWolf-Team wants to merge 1 commit into
Open
Conversation
…erver (CWE-248) The live-reload WebSocket message handler called JSON.parse() on incoming frames with no try/catch. Combined with the WebSocketServer accepting connections from any origin, any web page open in another browser tab (or any other cross-origin client) could crash the dev server by opening a WebSocket to it and sending a single non-JSON frame. Wrap JSON.parse() in a try/catch and silently ignore malformed messages instead of letting the exception propagate and terminate the process. Verified against the existing test suite (32/32 passing) plus a PoC that previously crashed the server with one malformed WebSocket frame. Co-Authored-By: iaohkut <thb2601@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #151 (the crash portion — see note on scope below).
Summary
The live-reload WebSocket message handler called
JSON.parse()on incoming frames with no try/catch. Combined with theWebSocketServeraccepting connections from any origin, any web page open in another browser tab (or any other cross-origin client) could crash the dev server by opening a WebSocket to it and sending a single non-JSON frame.Fix
Note on scope: #151 also flags the missing
Originvalidation on the WebSocket handshake as a contributing factor. I kept this fix scoped to the crash itself (which it fully closes on its own — no malformed message can bring the process down regardless of origin) rather than also adding origin-allowlisting, since the dev server has a documentedshowAllHostsoption for legitimate multi-host/LAN access, and getting an origin allowlist right without breaking that felt like a design decision better left to a maintainer's judgment than bundled into a minimal security patch. Happy to follow up with an origin-check PR if that's wanted.Verification
Full existing test suite: 32/32 passing, no regressions. PoC that previously crashed the server with a single malformed cross-origin WebSocket frame now leaves the server running normally.
See #151 for the full report and original reproduction.