Vulnerability details

### Summary A connection created with `forge.tls.createConnection()` throws an uncaught `TypeError` from `process()` when it receives a record of another content type while waiting for the rest of a handshake message that was split across records. A peer can trigger this with 18 bytes before the handshake, without authentication, which terminates Node.js processes that use the documented socket wiring. Fixed in node-forge 1.4.1. ### Details In `lib/tls.js`, `_readRecord()` handles a record that arrives while a handshake message split across records is incomplete. If the record has any other content type (change_cipher_spec, alert, application_data or heartbeat), forge correctly reports "Invalid fragmented record." through `c.error()` with a fatal `unexpected_message` alert. That fatal error closes and resets the connection, which sets `c.record` to `null`. `_readRecord()` then continues with `c.record.ready = true` and throws `TypeError: Cannot set properties of null (setting 'ready')` out of `process()`. The code is unchanged since the first release. RFC 5246 section 6.2.1 allows a handshake message to be fragmented across records, and RFC 8446 section 5.1 forbids interleaving handshake messages with other record types. Failing the connection with `unexpected_message` is the intended behavior; the bug is the crash after the error. node-forge reports TLS protocol errors through the `error` callback, so applications do not normally wrap `process()` in `try`/`catch`. When `process()` is called from a Node.js socket `data` handler, as in the documented usage, the exception is uncaught and terminates the process. The fix returns from `_readRecord()` right after the fatal error. The connection is failed, the alert is sent and the `error` callback is called as before, and `process()` no longer throws. ### Patches Fixed in node-forge 1.4.1. The connection still fails with a fatal `unexpected_message` alert and the `error` callback, but `process()` no longer throws. ### Workarounds Wrap every call to `connection.process()` in `try`/`catch`. The connection has already been failed and closed when the exception is thrown, so the exception can be ignored or the socket destroyed. ### Proof of concept Input (server mode), two records, 18 bytes: 16 03 01 00 06 01 00 00 10 03 01 handshake record: ClientHello header announcing 16 bytes, 2 present 15 03 01 00 02 02 28 alert record // c = forge.tls.createConnection({server: true, ...}) c.process(forge.util.hexToBytes('1603010006010000100301' + '15030100020228')); On 1.4.0 the `error` callback receives "Invalid fragmented record.", then `process()` throws `TypeError: Cannot set properties of null (setting 'ready')`. A change_cipher_spec, application_data or heartbeat record in place of the alert has the same effect, and a client connection behaves the same with a fragment of a ServerHello. In a Node.js server that calls `c.process(data.toString('binary'))` from `socket.on('data', ...)`, a client sending these 18 bytes terminates the server process. On 1.4.1 the connection fails with a fatal `unexpected_message` alert and `process()` returns normally. Regression tests in tests/unit/tls.js, "fragmented handshake messages": each of the four other record types after a handshake fragment, in server and client mode, plus same-type continuation records that are still accepted. ### Impact Applications that use node-forge's TLS implementation: connections from `forge.tls.createConnection()`, including those created by `forge.tls.wrapSocket()` and the `forge.http` / `forge.xhr` TLS connections. Any peer that can connect to a forge TLS server, or any server a forge TLS client connects to, can send 18 bytes before the handshake, without authentication. In Node.js applications using the documented usage, the uncaught exception terminates the process, which drops every connection it serves, and the attacker can repeat it on every restart. There is no confidentiality or integrity impact.