websocket: validate close() arguments and reject unmasked client frames

Merged
RO
2mo ago
+456-20

Four RFC 6455 / WHATWG conformance fixes in WebSocket close and frame handling, from a fuzzing report that cross-checked Bun against byte-level RFC 6455 peers, node, and ws.

1. WebSocket#close() had no argument validation

const ws = new WebSocket(url);
ws.close(5000);                  // no throw, 5000 goes on the wire (node/browsers: InvalidAccessError)
ws.close(1000, "R".repeat(124)); // no throw, and the reason is silently sent as empty

The spec check in WebSocket::close was commented out, and the reason-length check only ran when a code other than 1000 was passed, comparing UTF-16 code units instead of UTF-8 bytes. The native client then silently dropped reasons longer than 123 bytes before writing the frame.

close() now validates before any state transition:

  • a code an endpoint must not send (RFC 6455 7.4: anything outside 1000-1014 minus the reserved 1004-1006, or 3000-4999) throws an InvalidAccessError DOMException
  • a reason longer than 123 UTF-8 bytes throws a SyntaxError

One deliberate deviation from the browser whitelist (1000 or 3000-4999): the RFC endpoint codes 1001-1003 and 1007-1014 stay permitted. Bun's WebSocket already serves non-browser roles (ping(), pong(), terminate(), headers), the ws package (whose Bun shim forwards to this method) accepts exactly this set, and Bun's own inspector client closes with 1001. Tightening to the browser set would turn all of those into runtime throws. Every code the report flagged (999, 1005, 5000, and the over-long reason) is rejected either way. If full WHATWG strictness is preferred I can do that instead, but it needs a private path for the ws shim and a change to bun-inspector-protocol.

2. Bun.serve did not enforce client frame masking (RFC 6455 5.1 MUST)

The uWS parser computes the server's frame header size assuming the 4-byte masking key is present and never read the MASK bit. An unmasked client frame was parsed as if masked: the first payload bytes were consumed as the key, desyncing the frame stream, and an unmasked zero-length frame was delivered to message(). A server that mis-parses deliberately malformed frames instead of failing the connection is how request-smuggling-style bugs start.

The frame header validation now force-closes the connection when the mask bit does not match the role (Received an incorrectly masked frame is surfaced to the close handler), so a mismatched frame is never parsed or delivered. Bun's own WebSocket client always sets the mask bit, so Bun-to-Bun traffic is unaffected; this only rejects peers that violate the MUST.

3. Received close codes 1015 and 5000-65535 were surfaced verbatim

received_close_codes already mapped <1000, 1004-1006, and 1016-2999 to 1002 (protocol error) but missed 1015 (reserved, must not appear on the wire) and everything above 4999 (not defined). Both now map to 1002 like the rest, matching node and ws.

4. Invalid UTF-8 in a text message reported 1003 instead of 1007

RFC 6455 7.4.1: 1007 is "received data inconsistent with the type of the message (e.g. non-UTF-8 within a text message)"; 1003 means an unsupported data type. One constant in the error-code table.

Not addressed here

  • close() still fires the close event from within the call and never observably enters CLOSING; that rework is #27259.
  • On a protocol error (including invalid UTF-8), the client still hard-closes without sending a Close frame to the peer. Sending 1002/1007 first is a SHOULD and needs its own pass over the close state machine.
  • ServerWebSocket#close() reason truncation (wire vs local callback disagreement).

Tests

test/js/web/websocket/websocket-close-code.test.ts (new) covers close() argument validation (every rejected-code class, both reason-length failure modes plus the 123-byte boundary, validation ordering on CONNECTING and CLOSED sockets, and a wire round-trip for every permitted code) and the close-frame cases this PR changes, against a byte-level raw server: received 1015/5000/65535 report 1002, 1014 passes through, invalid UTF-8 reports 1007. The existing received-close-code matrix in websocket.test.js already covers the other reserved ranges and is untouched.

test/js/bun/websocket/websocket-server-unmasked-frames.test.ts (new) drives a raw TCP client through the upgrade and asserts unmasked text, empty, close, and lone 2-byte frames are never delivered and fail the connection, with masked and split-header controls.

With the fix reverted, 12 of the 27 new tests fail (no throw from close(5000), message("") delivered for an unmasked frame, code 5000 surfaced to JS, 1003 instead of 1007); all pass with it.