fix websocket receiving an invalid utf-8 in close frame (#3206)

* fix websocket receiving an invalid utf-8 in close frame

* fixup

* fixup

* fail if receiving masked frame

* fail on invalid status code in close frame

lib/web/websocket/connection.js

@@ -1,20 +1,22 @@
 'use strict'
 
-const { uid, states, sentCloseFrameState } = require('./constants')
+const { uid, states, sentCloseFrameState, emptyBuffer, opcodes } = require('./constants')
 const {
   kReadyState,
   kSentClose,
   kByteParser,
-  kReceivedClose
+  kReceivedClose,
+  kResponse
 } = require('./symbols')
-const { fireEvent, failWebsocketConnection } = require('./util')
+const { fireEvent, failWebsocketConnection, isClosing, isClosed, isEstablished } = require('./util')
 const { channels } = require('../../core/diagnostics')
 const { CloseEvent } = require('./events')
 const { makeRequest } = require('../fetch/request')
 const { fetching } = require('../fetch/index')
 const { Headers } = require('../fetch/headers')
 const { getDecodeSplit } = require('../fetch/util')
 const { kHeadersList } = require('../../core/symbols')
+const { WebsocketFrameSend } = require('./frame')
 
 /** @type {import('crypto')} */
 let crypto
@@ -211,6 +213,72 @@ function establishWebSocketConnection (url, protocols, ws, onEstablish, options)
   return controller
 }
 
+function closeWebSocketConnection (ws, code, reason, reasonByteLength) {
+  if (isClosing(ws) || isClosed(ws)) {
+    // If this's ready state is CLOSING (2) or CLOSED (3)
+    // Do nothing.
+  } else if (!isEstablished(ws)) {
+    // If the WebSocket connection is not yet established
+    // Fail the WebSocket connection and set this's ready state
+    // to CLOSING (2).
+    failWebsocketConnection(ws, 'Connection was closed before it was established.')
+    ws[kReadyState] = states.CLOSING
+  } else if (ws[kSentClose] === sentCloseFrameState.NOT_SENT) {
+    // If the WebSocket closing handshake has not yet been started
+    // Start the WebSocket closing handshake and set this's ready
+    // state to CLOSING (2).
+    // - If neither code nor reason is present, the WebSocket Close
+    //   message must not have a body.
+    // - If code is present, then the status code to use in the
+    //   WebSocket Close message must be the integer given by code.
+    // - If reason is also present, then reasonBytes must be
+    //   provided in the Close message after the status code.
+
+    ws[kSentClose] = sentCloseFrameState.PROCESSING
+
+    const frame = new WebsocketFrameSend()
+
+    // If neither code nor reason is present, the WebSocket Close
+    // message must not have a body.
+
+    // If code is present, then the status code to use in the
+    // WebSocket Close message must be the integer given by code.
+    if (code !== undefined && reason === undefined) {
+      frame.frameData = Buffer.allocUnsafe(2)
+      frame.frameData.writeUInt16BE(code, 0)
+    } else if (code !== undefined && reason !== undefined) {
+      // If reason is also present, then reasonBytes must be
+      // provided in the Close message after the status code.
+      frame.frameData = Buffer.allocUnsafe(2 + reasonByteLength)
+      frame.frameData.writeUInt16BE(code, 0)
+      // the body MAY contain UTF-8-encoded data with value /reason/
+      frame.frameData.write(reason, 2, 'utf-8')
+    } else {
+      frame.frameData = emptyBuffer
+    }
+
+    /** @type {import('stream').Duplex} */
+    const socket = ws[kResponse].socket
+
+    socket.write(frame.createFrame(opcodes.CLOSE), (err) => {
+      if (!err) {
+        ws[kSentClose] = sentCloseFrameState.SENT
+      }
+    })
+
+    ws[kSentClose] = sentCloseFrameState.PROCESSING
+
+    // Upon either sending or receiving a Close control frame, it is said
+    // that _The WebSocket Closing Handshake is Started_ and that the
+    // WebSocket connection is in the CLOSING state.
+    ws[kReadyState] = states.CLOSING
+  } else {
+    // Otherwise
+    // Set this's ready state to CLOSING (2).
+    ws[kReadyState] = states.CLOSING
+  }
+}
+
 /**
  * @param {Buffer} chunk
  */
@@ -237,10 +305,10 @@ function onSocketClose () {
 
   const result = ws[kByteParser].closingInfo
 
-  if (result) {
+  if (result && !result.error) {
     code = result.code ?? 1005
     reason = result.reason
-  } else if (ws[kSentClose] !== sentCloseFrameState.SENT) {
+  } else if (!ws[kReceivedClose]) {
     // If _The WebSocket
     // Connection is Closed_ and no Close control frame was received by the
     // endpoint (such as could occur if the underlying transport connection
@@ -293,5 +361,6 @@ function onSocketError (error) {
 }
 
 module.exports = {
-  establishWebSocketConnection
+  establishWebSocketConnection,
+  closeWebSocketConnection
 }

lib/web/websocket/receiver.js

@@ -6,6 +6,7 @@ const { kReadyState, kSentClose, kResponse, kReceivedClose } = require('./symbol
 const { channels } = require('../../core/diagnostics')
 const { isValidStatusCode, failWebsocketConnection, websocketMessageReceived, utf8Decode } = require('./util')
 const { WebsocketFrameSend } = require('./frame')
+const { CloseEvent } = require('./events')
 
 // This code was influenced by ws released under the MIT license.
 // Copyright (c) 2011 Einar Otto Stangvik <einaros@gmail.com>
@@ -55,6 +56,12 @@ class ByteParser extends Writable {
 
         this.#info.fin = (buffer[0] & 0x80) !== 0
         this.#info.opcode = buffer[0] & 0x0F
+        this.#info.masked = (buffer[1] & 0x80) === 0x80
+
+        if (this.#info.masked) {
+          failWebsocketConnection(this.ws, 'Frame cannot be masked')
+          return callback()
+        }
 
         // If we receive a fragmented message, we use the type of the first
         // frame to parse the full message as binary/text, when it's terminated
@@ -102,6 +109,13 @@ class ByteParser extends Writable {
 
           this.#info.closeInfo = this.parseCloseBody(body)
 
+          if (this.#info.closeInfo.error) {
+            const { code, reason } = this.#info.closeInfo
+
+            callback(new CloseEvent('close', { wasClean: false, reason, code }))
+            return
+          }
+
           if (this.ws[kSentClose] !== sentCloseFrameState.SENT) {
             // If an endpoint receives a Close frame and did not previously send a
             // Close frame, the endpoint MUST send a Close frame in response.  (When
@@ -310,16 +324,16 @@ class ByteParser extends Writable {
     }
 
     if (code !== undefined && !isValidStatusCode(code)) {
-      return null
+      return { code: 1002, reason: 'Invalid status code', error: true }
     }
 
     try {
       reason = utf8Decode(reason)
     } catch {
-      return null
+      return { code: 1007, reason: 'Invalid UTF-8', error: true }
     }
 
-    return { code, reason }
+    return { code, reason, error: false }
   }
 
   get closingInfo () {

lib/web/websocket/util.js

@@ -197,7 +197,8 @@ function failWebsocketConnection (ws, reason) {
   if (reason) {
     // TODO: process.nextTick
     fireEvent('error', ws, (type, init) => new ErrorEvent(type, init), {
-      error: new Error(reason)
+      error: new Error(reason),
+      message: reason
     })
   }
 }

lib/web/websocket/websocket.js

@@ -3,7 +3,7 @@
 const { webidl } = require('../fetch/webidl')
 const { URLSerializer } = require('../fetch/data-url')
 const { getGlobalOrigin } = require('../fetch/global')
-const { staticPropertyDescriptors, states, sentCloseFrameState, opcodes, emptyBuffer } = require('./constants')
+const { staticPropertyDescriptors, states, sentCloseFrameState, opcodes } = require('./constants')
 const {
   kWebSocketURL,
   kReadyState,
@@ -16,18 +16,17 @@ const {
 const {
   isConnecting,
   isEstablished,
-  isClosed,
   isClosing,
   isValidSubprotocol,
-  failWebsocketConnection,
   fireEvent
 } = require('./util')
-const { establishWebSocketConnection } = require('./connection')
+const { establishWebSocketConnection, closeWebSocketConnection } = require('./connection')
 const { WebsocketFrameSend } = require('./frame')
 const { ByteParser } = require('./receiver')
 const { kEnumerableProperty, isBlobLike } = require('../../core/util')
 const { getGlobalDispatcher } = require('../../global')
 const { types } = require('node:util')
+const { ErrorEvent } = require('./events')
 
 let experimentalWarned = false
 
@@ -197,67 +196,7 @@ class WebSocket extends EventTarget {
     }
 
     // 3. Run the first matching steps from the following list:
-    if (isClosing(this) || isClosed(this)) {
-      // If this's ready state is CLOSING (2) or CLOSED (3)
-      // Do nothing.
-    } else if (!isEstablished(this)) {
-      // If the WebSocket connection is not yet established
-      // Fail the WebSocket connection and set this's ready state
-      // to CLOSING (2).
-      failWebsocketConnection(this, 'Connection was closed before it was established.')
-      this[kReadyState] = WebSocket.CLOSING
-    } else if (this[kSentClose] === sentCloseFrameState.NOT_SENT) {
-      // If the WebSocket closing handshake has not yet been started
-      // Start the WebSocket closing handshake and set this's ready
-      // state to CLOSING (2).
-      // - If neither code nor reason is present, the WebSocket Close
-      //   message must not have a body.
-      // - If code is present, then the status code to use in the
-      //   WebSocket Close message must be the integer given by code.
-      // - If reason is also present, then reasonBytes must be
-      //   provided in the Close message after the status code.
-
-      this[kSentClose] = sentCloseFrameState.PROCESSING
-
-      const frame = new WebsocketFrameSend()
-
-      // If neither code nor reason is present, the WebSocket Close
-      // message must not have a body.
-
-      // If code is present, then the status code to use in the
-      // WebSocket Close message must be the integer given by code.
-      if (code !== undefined && reason === undefined) {
-        frame.frameData = Buffer.allocUnsafe(2)
-        frame.frameData.writeUInt16BE(code, 0)
-      } else if (code !== undefined && reason !== undefined) {
-        // If reason is also present, then reasonBytes must be
-        // provided in the Close message after the status code.
-        frame.frameData = Buffer.allocUnsafe(2 + reasonByteLength)
-        frame.frameData.writeUInt16BE(code, 0)
-        // the body MAY contain UTF-8-encoded data with value /reason/
-        frame.frameData.write(reason, 2, 'utf-8')
-      } else {
-        frame.frameData = emptyBuffer
-      }
-
-      /** @type {import('stream').Duplex} */
-      const socket = this[kResponse].socket
-
-      socket.write(frame.createFrame(opcodes.CLOSE), (err) => {
-        if (!err) {
-          this[kSentClose] = sentCloseFrameState.SENT
-        }
-      })
-
-      // Upon either sending or receiving a Close control frame, it is said
-      // that _The WebSocket Closing Handshake is Started_ and that the
-      // WebSocket connection is in the CLOSING state.
-      this[kReadyState] = states.CLOSING
-    } else {
-      // Otherwise
-      // Set this's ready state to CLOSING (2).
-      this[kReadyState] = WebSocket.CLOSING
-    }
+    closeWebSocketConnection(this, code, reason, reasonByteLength)
   }
 
   /**
@@ -521,9 +460,8 @@ class WebSocket extends EventTarget {
     this[kResponse] = response
 
     const parser = new ByteParser(this)
-    parser.on('drain', function onParserDrain () {
-      this.ws[kResponse].socket.resume()
-    })
+    parser.on('drain', onParserDrain)
+    parser.on('error', onParserError.bind(this))
 
     response.socket.ws = this
     this[kByteParser] = parser
@@ -647,6 +585,16 @@ webidl.converters.WebSocketSendData = function (V) {
   return webidl.converters.USVString(V)
 }
 
+function onParserDrain () {
+  this.ws[kResponse].socket.resume()
+}
+
+function onParserError (err) {
+  fireEvent('error', this, () => new ErrorEvent('error', { error: err, message: err.reason }))
+
+  closeWebSocketConnection(this, err.code)
+}
+
 module.exports = {
   WebSocket
 }

test/websocket/client-received-masked-frame.js

@@ -0,0 +1,45 @@
+'use strict'
+
+const { test } = require('node:test')
+const { once } = require('node:events')
+const { WebSocketServer } = require('ws')
+const { WebSocket } = require('../..')
+const { tspl } = require('@matteo.collina/tspl')
+const { WebsocketFrameSend } = require('../../lib/web/websocket/frame')
+
+test('Client fails the connection if receiving a masked frame', async (t) => {
+  const assert = tspl(t, { plan: 2 })
+
+  const body = Buffer.allocUnsafe(2)
+  body.writeUInt16BE(1006, 0)
+
+  const frame = new WebsocketFrameSend(body)
+  const buffer = frame.createFrame(0x8)
+
+  const server = new WebSocketServer({ port: 0 })
+
+  server.on('connection', (ws) => {
+    const socket = ws._socket
+
+    socket.write(buffer, () => ws.close())
+  })
+
+  const ws = new WebSocket(`ws://localhost:${server.address().port}`)
+
+  ws.addEventListener('close', (e) => {
+    assert.deepStrictEqual(e.code, 1006)
+  })
+
+  ws.addEventListener('error', () => {
+    assert.ok(true)
+  })
+
+  t.after(() => {
+    server.close()
+    ws.close()
+  })
+
+  await once(ws, 'close')
+
+  await assert.completed
+})

test/websocket/close-invalid-status-code.js

@@ -0,0 +1,39 @@
+'use strict'
+
+const { test } = require('node:test')
+const { once } = require('node:events')
+const { WebSocketServer } = require('ws')
+const { WebSocket } = require('../..')
+const { tspl } = require('@matteo.collina/tspl')
+
+test('Client fails the connection if receiving a masked frame', async (t) => {
+  const assert = tspl(t, { plan: 2 })
+
+  const server = new WebSocketServer({ port: 0 })
+
+  server.on('connection', (ws) => {
+    const socket = ws._socket
+
+    // 1006 status code
+    socket.write(Buffer.from([0x88, 0x02, 0x03, 0xee]), () => ws.close())
+  })
+
+  const ws = new WebSocket(`ws://localhost:${server.address().port}`)
+
+  ws.addEventListener('close', (e) => {
+    assert.deepStrictEqual(e.code, 1006)
+  })
+
+  ws.addEventListener('error', () => {
+    assert.ok(true)
+  })
+
+  t.after(() => {
+    server.close()
+    ws.close()
+  })
+
+  await once(ws, 'close')
+
+  await assert.completed
+})