Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/docs/api/Client.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ Returns: `Client`
* **keepAliveTimeoutThreshold** `number | null` (optional) - Default: `2e3` - A number of milliseconds subtracted from server *keep-alive* hints when overriding `keepAliveTimeout` to account for timing inaccuracies caused by e.g. transport latency. Defaults to 2 seconds.
* **maxHeaderSize** `number | null` (optional) - Default: `--max-http-header-size` or `16384` - The maximum length of request headers in bytes. Defaults to Node.js' --max-http-header-size or 16KiB.
* **maxResponseSize** `number | null` (optional) - Default: `-1` - The maximum length of response body in bytes. Set to `-1` to disable.
* **webSocket** `WebSocketOptions` (optional) - WebSocket-specific configuration options.
* **maxPayloadSize** `number` (optional) - Default: `134217728` (128 MB) - Maximum allowed payload size in bytes for WebSocket messages. Applied to both uncompressed and decompressed (permessage-deflate) messages. Set to 0 to disable the limit.
* **pipelining** `number | null` (optional) - Default: `1` - The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Carefully consider your workload and environment before enabling concurrent requests as pipelining may reduce performance if used incorrectly. Pipelining is sensitive to network stack settings as well as head of line blocking caused by e.g. long running requests. Set to `0` to disable keep-alive connections.
* **connect** `ConnectOptions | Function | null` (optional) - Default: `null`.
* **strictContentLength** `Boolean` (optional) - Default: `true` - Whether to treat request content length mismatches as errors. If true, an error is thrown when the request content-length header doesn't match the length of the request body. **Security Warning:** Disabling this option can expose your application to HTTP Request Smuggling attacks, where mismatched content-length headers cause servers and proxies to interpret request boundaries differently. This can lead to cache poisoning, credential hijacking, and bypassing security controls. Only disable this in controlled environments where you fully trust the request source.
Expand Down
2 changes: 1 addition & 1 deletion lib/dispatcher/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Agent extends DispatcherBase {
throw new InvalidArgumentError('maxOrigins must be a number greater than 0')
}

super()
super(options)

if (connect && typeof connect !== 'function') {
connect = { ...connect }
Expand Down
5 changes: 3 additions & 2 deletions lib/dispatcher/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ class Client extends DispatcherBase {
useH2c,
initialWindowSize,
connectionWindowSize,
pingInterval
pingInterval,
webSocket
} = {}) {
if (keepAlive !== undefined) {
throw new InvalidArgumentError('unsupported keepAlive, use pipelining=0 instead')
Expand Down Expand Up @@ -222,7 +223,7 @@ class Client extends DispatcherBase {
throw new InvalidArgumentError('pingInterval must be a positive integer, greater or equal to 0')
}

super()
super({ webSocket })

if (typeof connect !== 'function') {
connect = buildConnector({
Expand Down
18 changes: 18 additions & 0 deletions lib/dispatcher/dispatcher-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const { kDestroy, kClose, kClosed, kDestroyed, kDispatch } = require('../core/sy

const kOnDestroyed = Symbol('onDestroyed')
const kOnClosed = Symbol('onClosed')
const kWebSocketOptions = Symbol('webSocketOptions')

class DispatcherBase extends Dispatcher {
/** @type {boolean} */
Expand All @@ -25,6 +26,23 @@ class DispatcherBase extends Dispatcher {
/** @type {Array<Function>|null} */
[kOnClosed] = null

/**
* @param {import('../../types/dispatcher').DispatcherOptions} [opts]
*/
constructor (opts) {
super()
this[kWebSocketOptions] = opts?.webSocket ?? {}
}

/**
* @returns {import('../../types/dispatcher').WebSocketOptions}
*/
get webSocketOptions () {
return {
maxPayloadSize: this[kWebSocketOptions].maxPayloadSize ?? 128 * 1024 * 1024 // 128 MB default
}
}

/** @returns {boolean} */
get destroyed () {
return this[kDestroyed]
Expand Down
2 changes: 1 addition & 1 deletion lib/dispatcher/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class Pool extends PoolBase {
})
}

super()
super(options)

this[kConnections] = connections || null
this[kUrl] = util.parseOrigin(origin)
Expand Down
29 changes: 24 additions & 5 deletions lib/web/websocket/permessage-deflate.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,46 @@ const tail = Buffer.from([0x00, 0x00, 0xff, 0xff])
const kBuffer = Symbol('kBuffer')
const kLength = Symbol('kLength')

// Default maximum decompressed message size: 4 MB
const kDefaultMaxDecompressedSize = 4 * 1024 * 1024
// Default maximum decompressed message size: 128 MB
const kDefaultMaxDecompressedSize = 128 * 1024 * 1024

class PerMessageDeflate {
/** @type {import('node:zlib').InflateRaw} */
#inflate

#options = {}

/** @type {number} */
#maxPayloadSize

/** @type {boolean} */
#aborted = false

/** @type {Function|null} */
#currentCallback = null

/** @type {number} */
#currentBufferedBytes = 0

/**
* @param {Map<string, string>} extensions
* @param {{ maxPayloadSize?: number }} [options]
*/
constructor (extensions) {
constructor (extensions, options = {}) {
this.#options.serverNoContextTakeover = extensions.has('server_no_context_takeover')
this.#options.serverMaxWindowBits = extensions.get('server_max_window_bits')
// 0 disables the limit
this.#maxPayloadSize = options.maxPayloadSize ?? kDefaultMaxDecompressedSize
}

decompress (chunk, fin, callback) {
/**
* Decompress a compressed payload.
* @param {Buffer} chunk Compressed data
* @param {boolean} fin Final fragment flag
* @param {Function} callback Callback function
* @param {number} [bufferedBytes] Bytes already accumulated for the current message
*/
decompress (chunk, fin, callback, bufferedBytes = 0) {
// An endpoint uses the following algorithm to decompress a message.
// 1. Append 4 octets of 0x00 0x00 0xff 0xff to the tail end of the
// payload of the message.
Expand Down Expand Up @@ -70,7 +86,8 @@ class PerMessageDeflate {

this.#inflate[kLength] += data.length

if (this.#inflate[kLength] > kDefaultMaxDecompressedSize) {
// 0 disables the limit
if (this.#maxPayloadSize > 0 && this.#currentBufferedBytes + this.#inflate[kLength] > this.#maxPayloadSize) {
this.#aborted = true
this.#inflate.removeAllListeners()
this.#inflate.destroy()
Expand All @@ -94,6 +111,7 @@ class PerMessageDeflate {
}

this.#currentCallback = callback
this.#currentBufferedBytes = bufferedBytes
this.#inflate.write(chunk)
if (fin) {
this.#inflate.write(tail)
Expand All @@ -109,6 +127,7 @@ class PerMessageDeflate {
this.#inflate[kBuffer].length = 0
this.#inflate[kLength] = 0
this.#currentCallback = null
this.#currentBufferedBytes = 0

callback(null, full)
})
Expand Down
80 changes: 58 additions & 22 deletions lib/web/websocket/receiver.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,23 @@ class ByteParser extends Writable {
/** @type {import('./websocket').Handler} */
#handler

/** @type {number} */
#maxPayloadSize

/**
* @param {import('./websocket').Handler} handler
* @param {Map<string, string>|null} extensions
* @param {{ maxPayloadSize?: number }} [options]
*/
constructor (handler, extensions) {
constructor (handler, extensions, options = {}) {
super()

this.#handler = handler
this.#extensions = extensions == null ? new Map() : extensions
this.#maxPayloadSize = options.maxPayloadSize ?? 0

if (this.#extensions.has('permessage-deflate')) {
this.#extensions.set('permessage-deflate', new PerMessageDeflate(extensions))
this.#extensions.set('permessage-deflate', new PerMessageDeflate(extensions, options))
}
}

Expand All @@ -66,6 +71,20 @@ class ByteParser extends Writable {
this.run(callback)
}

#validatePayloadLength () {
if (
this.#maxPayloadSize > 0 &&
!isControlFrame(this.#info.opcode) &&
!this.#info.compressed &&
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we ignore compressed messages here, but then perform the same logic in the permessage-deflate handler?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are doing this kind of check in 3 places:

  1. here for normal packets
  2. early rejection for compressed packets (estimating a 10x expansion rate, we can avoid trying to decompress things, saves CPU)
  3. actual/precise validation for compressed packages

The 2nd could be optional, but it saves CPU cycles.

Copy link
Copy Markdown
Member

@KhafraDev KhafraDev Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we're skipping compressed packets here, it's not much of an early return if you still have to go all the way to the decompression handler

what I'm asking for is why we're skipping compressed packets here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are skipping compressed packets here because it's a different logic as @tsctx has asked.

Copy link
Copy Markdown
Member

@KhafraDev KhafraDev Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is not different.

Assuming we send an uncompressed and compressed frame that's 129MB:

  • Uncompressed: early exits here
  • Compressed: skips this, goes through READ_DATA, goes to permessage deflate handler, attempt to inflate, if payload size > max payload size, exit

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that, but why skip them? Couldn't an attacker just send a compressed frame that has an unlimited size, where undici will collect all fragments, attempt to inflate, then fail much later?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check is done chunk by chunk on compression, as soon as it goes over the limit, the connection is interrupted.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve looked into it, and while we check the max length per message unit, we seem to be missing a check for the total buffered size of fragmented messages.

We don't actually need a separate check for compression; we should simply trigger an early exit if the accumulated size plus the current payload exceeds the limit. Even with compressed data, we can perform this early check if the payload itself is already too large.

This is already handled for uncompressed fragments, but for the compressed path, we should also pass the current buffered size to the decompression side to ensure the same validation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've written some pseudo-code to illustrate the issue more clearly.

createServer((socket) => {
    // Start by fragmenting the message.
    // At this stage, 64MB has been accumulated in the buffer.
    socket.send(alloc(64 * 1024 * 1024), { fin: false }); 
    
    // Case 1: Early termination is not triggered because compressed messages 
    // are currently being skipped.
    // The pre-compressed size is 100MB. Since 64MB is already buffered, 
    // it clearly exceeds the limit and should be terminated immediately.
    socket.send(alloc_post_compressed(100 * 1024 * 1024), { fin: true }); 
    // alloc_post_compressed: The size represents the "expected final output."

    // Case 2: The current accumulated size is not passed to the decompressor, 
    // so this segment is treated as only 100MB, bypassing the decompression limit.
    // We should pass the current buffer size to the decompressor and 
    // validate the total size (current buffer + decompressed payload).
    socket.send(alloc_pre_compressed(100 * 1024 * 1024), { fin: true }); 
    // alloc_pre_compressed: The size represents the "actual memory footprint."
});

Copy link
Copy Markdown
Member

@KhafraDev KhafraDev Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @tsctx this is what I was trying to articulate, lol.

The issue is if the compressed data is larger than the maxPayloadSize, we shouldn't bother going to READ_DATA and inflating the data which is already larger than the maxPayloadSize.

The permessage-deflate handler catches it, once the decompressed data is larger than the maxPayloadSize. This only happens once we have a full frame, so an attacker could theoretically send n fragmented frames each with unlimited size that are kept in memory until we receive the entirety of the frame.

From READ_DATA, should be helpful in understanding:

        if (this.#byteOffset < this.#info.payloadLength) {
          return callback()
        }

        const body = this.consume(this.#info.payloadLength)

this.#fragmentsBytes + this.#info.payloadLength > this.#maxPayloadSize
) {
failWebsocketConnection(this.#handler, 1009, 'Payload size exceeds maximum allowed size')
return false
}

return true
}

/**
* Runs whenever a new chunk is received.
* Callback is called whenever there are no more chunks buffering,
Expand Down Expand Up @@ -169,6 +188,10 @@ class ByteParser extends Writable {
this.#info.masked = masked
this.#info.fin = fin
this.#info.fragmented = fragmented

if (this.#state === parserStates.READ_DATA && !this.#validatePayloadLength()) {
return
}
} else if (this.#state === parserStates.PAYLOADLENGTH_16) {
if (this.#byteOffset < 2) {
return callback()
Expand All @@ -178,6 +201,10 @@ class ByteParser extends Writable {

this.#info.payloadLength = buffer.readUInt16BE(0)
this.#state = parserStates.READ_DATA

if (!this.#validatePayloadLength()) {
return
}
} else if (this.#state === parserStates.PAYLOADLENGTH_64) {
if (this.#byteOffset < 8) {
return callback()
Expand All @@ -200,6 +227,10 @@ class ByteParser extends Writable {

this.#info.payloadLength = lower
this.#state = parserStates.READ_DATA

if (!this.#validatePayloadLength()) {
return
}
} else if (this.#state === parserStates.READ_DATA) {
if (this.#byteOffset < this.#info.payloadLength) {
return callback()
Expand All @@ -224,29 +255,34 @@ class ByteParser extends Writable {

this.#state = parserStates.INFO
} else {
this.#extensions.get('permessage-deflate').decompress(body, this.#info.fin, (error, data) => {
if (error) {
// Use 1009 (Message Too Big) for decompression size limit errors
const code = error instanceof MessageSizeExceededError ? 1009 : 1007
failWebsocketConnection(this.#handler, code, error.message)
return
}

this.writeFragments(data)
this.#extensions.get('permessage-deflate').decompress(
body,
this.#info.fin,
(error, data) => {
if (error) {
// Use 1009 (Message Too Big) for decompression size limit errors
const code = error instanceof MessageSizeExceededError ? 1009 : 1007
failWebsocketConnection(this.#handler, code, error.message)
return
}

this.writeFragments(data)

if (!this.#info.fin) {
this.#state = parserStates.INFO
this.#loop = true
this.run(callback)
return
}

websocketMessageReceived(this.#handler, this.#info.binaryType, this.consumeFragments())

if (!this.#info.fin) {
this.#state = parserStates.INFO
this.#loop = true
this.#state = parserStates.INFO
this.run(callback)
return
}

websocketMessageReceived(this.#handler, this.#info.binaryType, this.consumeFragments())

this.#loop = true
this.#state = parserStates.INFO
this.run(callback)
})
},
this.#fragmentsBytes
)

this.#loop = false
break
Expand Down
7 changes: 6 additions & 1 deletion lib/web/websocket/websocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,12 @@ class WebSocket extends EventTarget {
// once this happens, the connection is open
this.#handler.socket = response.socket

const parser = new ByteParser(this.#handler, parsedExtensions)
// Get maxPayloadSize from dispatcher options
const maxPayloadSize = this.#handler.controller.dispatcher?.webSocketOptions?.maxPayloadSize

const parser = new ByteParser(this.#handler, parsedExtensions, {
maxPayloadSize
})
parser.on('drain', () => this.#handler.onParserDrain())
parser.on('error', (err) => this.#handler.onParserError(err))

Expand Down
Loading
Loading