From bd2cdc7184940db091363cacb14288b9f51d512a Mon Sep 17 00:00:00 2001 From: Shreemaan Abhishek Date: Tue, 22 Sep 2026 18:56:23 +0545 Subject: [PATCH] bugfix: client: bound the handshake response header read The handshake response header block was read with an unbounded receiveuntil("\r\n\r\n"), so a malicious or broken server could stream bytes forever without ever sending the terminator and the cosocket would buffer all of it, exhausting the nginx worker's memory and taking down every request it is handling. Read at most max_header_len bytes (8192 by default, mirroring nginx's own header buffer) and refuse anything larger. Set max_header_len = 0 to restore the unbounded read. The same cap covers the CONNECT response read on the wss-over-proxy path, which had the same unbounded read. --- README.markdown | 3 + lib/resty/websocket/client.lua | 51 ++++++-- t/max_header_len.t | 224 +++++++++++++++++++++++++++++++++ 3 files changed, 271 insertions(+), 7 deletions(-) create mode 100644 t/max_header_len.t diff --git a/README.markdown b/README.markdown index d899a28..35ea004 100644 --- a/README.markdown +++ b/README.markdown @@ -364,6 +364,9 @@ An optional options table can be specified. The following options are as follows * `max_send_len` Specifies the maximal length of payload allowed when sending WebSocket frames. Defaults to the value of `max_payload_len`. +* `max_header_len` + + Specifies the maximal size, in bytes, of the response header block read during the handshake. A server that sends more than this, or that never terminates the header block at all, is refused instead of being buffered. Defaults to `8192`. Set it to `0` to read without a limit. * `send_unmasked` Specifies whether to send out an unmasked WebSocket frames. When it is `true`, unmasked frames are always sent. Default to `false`. RFC 6455 requires, however, that the client MUST send masked frames to the server, so never set this option to `true` unless you know what you are doing. diff --git a/lib/resty/websocket/client.lua b/lib/resty/websocket/client.lua index bb95958..ce7823d 100644 --- a/lib/resty/websocket/client.lua +++ b/lib/resty/websocket/client.lua @@ -48,6 +48,37 @@ _M._VERSION = '0.13' local mt = { __index = _M } +-- default cap on the size of a handshake response header block, in bytes; +-- mirrors nginx's own 8k header buffer +local DEFAULT_MAX_HEADER_LEN = 8192 + + +-- reads the response header block terminated by CRLFCRLF, refusing to buffer +-- more than max_header_len bytes. max_header_len == 0 means no limit. +local function recv_header(sock, max_header_len) + local reader = sock:receiveuntil("\r\n\r\n") + + if max_header_len == 0 then + return reader() + end + + -- ask for one byte past the limit: a header block that fits comes back + -- whole, while an oversized one (or a peer that never terminates the + -- block at all) comes back at the limit instead of being buffered forever + local header, err, partial = reader(max_header_len + 1) + if not header then + return nil, err, partial + end + + if #header > max_header_len then + return nil, "response headers too large (limit: " + .. max_header_len .. " bytes)" + end + + return header +end + + function _M.new(self, opts) local sock, err = tcp() if not sock then @@ -55,11 +86,18 @@ function _M.new(self, opts) end local max_payload_len, send_unmasked, timeout - local max_recv_len, max_send_len + local max_recv_len, max_send_len, max_header_len if opts then max_payload_len = opts.max_payload_len max_recv_len = opts.max_recv_len max_send_len = opts.max_send_len + max_header_len = opts.max_header_len + + if max_header_len ~= nil + and (type(max_header_len) ~= "number" or max_header_len < 0) + then + return nil, "max_header_len must be a non-negative number" + end send_unmasked = opts.send_unmasked timeout = opts.timeout @@ -72,11 +110,13 @@ function _M.new(self, opts) max_payload_len = max_payload_len or 65535 max_recv_len = max_recv_len or max_payload_len max_send_len = max_send_len or max_payload_len + max_header_len = max_header_len or DEFAULT_MAX_HEADER_LEN return setmetatable({ sock = sock, max_recv_len = max_recv_len, max_send_len = max_send_len, + max_header_len = max_header_len, send_unmasked = send_unmasked, }, mt) end @@ -288,9 +328,7 @@ function _M.connect(self, uri, opts) return nil, "failed to send the handshake request: " .. err end - local header_reader = sock:receiveuntil("\r\n\r\n") - -- FIXME: check for too big response headers - local header, err, _ = header_reader() + local header, err = recv_header(sock, self.max_header_len) if not header then return nil, "failed to receive response header: " .. err end @@ -360,9 +398,8 @@ function _M.connect(self, uri, opts) return nil, "failed to send the handshake request: " .. err end - local header_reader = sock:receiveuntil("\r\n\r\n") - -- FIXME: check for too big response headers - local header, err, partial = header_reader() + local header + header, err = recv_header(sock, self.max_header_len) if not header then return nil, "failed to receive response header: " .. err end diff --git a/t/max_header_len.t b/t/max_header_len.t new file mode 100644 index 0000000..f156651 --- /dev/null +++ b/t/max_header_len.t @@ -0,0 +1,224 @@ +# vim:set ft= ts=4 sw=4 et: + +use Test::Nginx::Socket::Lua; +use Cwd qw(cwd); + +repeat_each(2); + +plan tests => repeat_each() * (3 * blocks()); + +my $pwd = cwd(); + +our $HttpConfig = qq{ + lua_package_path "$pwd/lib/?.lua;;"; + lua_package_cpath "/usr/local/openresty-debug/lualib/?.so;/usr/local/openresty/lualib/?.so;;"; +}; + +# a valid handshake response for the fixed key used below, padded past the +# 8192 byte default limit +our $BigReply = "HTTP/1.1 101 Switching Protocols\r +Upgrade: websocket\r +Connection: Upgrade\r +Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r +X-Padding: " . ("a" x 9000) . "\r +\r +"; + +our $SmallReply = "HTTP/1.1 101 Switching Protocols\r +Upgrade: websocket\r +Connection: Upgrade\r +Sec-WebSocket-Accept: s3pPLMBiTxaQ9kYGzzhZRbK+xOo=\r +\r +"; + +# never terminated by CRLFCRLF: the client must stop reading at the limit +our $UnterminatedReply = "HTTP/1.1 101 Switching Protocols\r +X-Padding: " . ("a" x 9000); + +no_long_string(); + +run_tests(); + +__DATA__ + +=== TEST 1: oversized response headers are refused by default +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local uri = "ws://127.0.0.1:7986/" + local ok, err = wb:connect(uri, + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval: $::BigReply +--- response_body +failed to connect: failed to receive response header: response headers too large (limit: 8192 bytes) +--- no_error_log +[error] + + + +=== TEST 2: max_header_len = 0 restores the unbounded read +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new{ max_header_len = 0 } + + local uri = "ws://127.0.0.1:7986/" + local ok, err = wb:connect(uri, + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval: $::BigReply +--- response_body +connected +--- no_error_log +[error] + + + +=== TEST 3: a smaller limit refuses a response that would otherwise fit +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new{ max_header_len = 32 } + + local uri = "ws://127.0.0.1:7986/" + local ok, err = wb:connect(uri, + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval: $::SmallReply +--- response_body +failed to connect: failed to receive response header: response headers too large (limit: 32 bytes) +--- no_error_log +[error] + + + +=== TEST 4: a header block that is never terminated is refused +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local uri = "ws://127.0.0.1:7986/" + local ok, err = wb:connect(uri, + { key = "dGhlIHNhbXBsZSBub25jZQ==" }) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + } + } +--- request +GET /t +--- tcp_listen: 7986 +--- tcp_reply eval: $::UnterminatedReply +--- response_body +failed to connect: failed to receive response header: response headers too large (limit: 8192 bytes) +--- no_error_log +[error] + + + +=== TEST 5: new() rejects a bad max_header_len +--- http_config eval: $::HttpConfig +--- config + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + + local wb, err = client:new{ max_header_len = -1 } + ngx.say("negative: ", wb, ", ", err) + + wb, err = client:new{ max_header_len = "8192" } + ngx.say("string: ", wb, ", ", err) + } + } +--- request +GET /t +--- response_body +negative: nil, max_header_len must be a non-negative number +string: nil, max_header_len must be a non-negative number +--- no_error_log +[error] + + + +=== TEST 6: a real handshake still succeeds under the default limit +--- http_config eval: $::HttpConfig +--- config + location = /ws { + content_by_lua_block { + local server = require "resty.websocket.server" + local wb, err = server:new() + if not wb then + ngx.log(ngx.ERR, "failed to new websocket: ", err) + return ngx.exit(444) + end + wb:recv_frame() + } + } + + location = /t { + content_by_lua_block { + local client = require "resty.websocket.client" + local wb = client:new() + + local uri = "ws://127.0.0.1:" .. ngx.var.server_port .. "/ws" + local ok, err = wb:connect(uri) + if not ok then + ngx.say("failed to connect: ", err) + return + end + + ngx.say("connected") + wb:close() + } + } +--- request +GET /t +--- response_body +connected +--- no_error_log +[error]