Ping Frames with no data

[cookiengineer]

These are the relevant spec sections:
https://tools.ietf.org/html/rfc6455#section-5.2
https://tools.ietf.org/html/rfc6455#section-5.3

The spec says that the mask key must not influence the payload data and length. From the section "5.2. Base Framing Protocol", you can see that the masking key is meant to be optional.

So I think the library has currently an incorrect behaviour for the ping frames, assuming that ping frames can be sent without any data. (I might be wrong, please correct me if so!)

The issue is in line 235 in WebSocketFrame.js:

    var output = new Buffer(this.length + headerLength + (this.mask ? 4 : 0));

The output buffer will be longer than its data (because this.length is 0 in an empty ping frame with no payload data), because the mask will never be written in it.

So the current behaviour is that it might get asynchronous by the 4 bytes of the mask if an empty ping frame is sent.

[theturtle32]

According to Section 5.2 of the specification, all frames from client to server have the mask bit set to 1:

   Mask:  1 bit

      Defines whether the "Payload data" is masked.  If set to 1, a
      masking key is present in masking-key, and this is used to unmask
      the "Payload data" as per Section 5.3.  All frames sent from
      client to server have this bit set to 1.

That means that it is not optional. Ping frames from client to server with no payload will still always have a 4-byte mask. It's true that later on in the code that those last 4 bytes are not written if the payload length is zero, but that won't cause loss of sync in the parser on the receiving end, because there will be four bytes there where it's expecting to find the mask key. Those bytes will be uninitialized, and so will be random bytes from the heap. So while that won't cause any errors, and is correct behavior according to the spec, that's actually a potential security vulnerability, as the contents of four bytes of random memory locations will be transmitted with each ping.

[theturtle32]

I don't think this is a high-priority security issue, however, because it only applies when using Node as a WebSocket client, not as a server, because servers don't mask any frames sent to the client. Comparatively few people, to the best of my knowledge, are using node as a WebSocket client to connect to an untrusted or potentially malicious server over a public network.

[theturtle32]

Fixed and released as v1.0.16