MINOR: payload: make smp_fetch_ssl_hello_alpn() use smp_client_hello_parse()
Same rationale as the previous commit for smp_fetch_ssl_hello_sni():
avoid duplicating the ClientHello parsing already factored out in
smp_client_hello_parse(). No behavior change.
MINOR: payload: make smp_fetch_ssl_hello_sni() use smp_client_hello_parse()
Avoid duplicating the ClientHello parsing already factored out in
smp_client_hello_parse(), like the other req_ssl_*/ssl_* fetches
already do. No behavior change.
BUG/MINOR: payload: fix handshake length off-by-4 in ssl_hello_sni/alpn
smp_fetch_ssl_hello_sni() and smp_fetch_ssl_hello_alpn() each have
their own copy of the ClientHello preamble parser instead of using the
shared smp_client_hello_parse(). The handshake length ("hs_len") is
compared against "bleft" before "bleft" is decremented for the 4-byte
handshake header (msg_type + 3-byte length), so a ClientHello claiming
a body up to 4 bytes larger than what was actually sent is still
accepted as complete. Every subsequent length-derived bound inherits
this same 4-byte over-count, letting the final read (SNI hostname or
ALPN protocol name) run up to 4 bytes past the received buffer and
disclose uninitialized memory through req.ssl_sni / req.ssl_alpn.
This is the same issue already fixed in smp_client_hello_parse() by
commit 2653936510 ("BUG/MINOR: payload: fix the handshake length
bounds check smp_client_hello_parse()"), just never ported to these
two functions' own duplicated preamble. This applies the same fix:
"data += 4" now comes with "bleft -= 4" before the length check
instead of after it, and the record-layer length is checked as soon
as the handshake is entered.
This must be backported to all supported versions.
Willy Tarreau [Tue, 11 Aug 2026 11:56:44 +0000 (13:56 +0200)]
MINOR: debug: force complain's backtrace to be a tail jump instead of a tail call
We can make complain_with_dbg() and complain() disappear from the
backtrace by forcing tail call elimination on these functions, since
ha_backtrace_to_stderr() is called last and with a compatible return
code.
Willy Tarreau [Tue, 11 Aug 2026 13:14:31 +0000 (15:14 +0200)]
MINOR: compiler: add a function attribute to disable tail calls
In some cases we want a tail call to be turned into a jump, which is
sometimes referred to as "tail call elimination". Gcc and clang have
options for this, so let's add an attribute "eliminate_tail_calls" that
works with both.
Willy Tarreau [Tue, 11 Aug 2026 11:43:46 +0000 (13:43 +0200)]
MEDIUM: debug: merge the counter and message emission into complain()
The event counter in debug mode moved to complain_with_dbg(). This
ensures that the deferencing of the counter, the atomic ops, the
condition to skip the emission are all centralized and are no longer
inline in the BUG_ON() location.
The trace emission that was now systematic after calls to complain*()
now moved to the shared _complain(), where it is emitted based on the
elements present in the details passed to the function (fatal/warning).
This saves another 56kB, so we're at -193kB compared to initial work.
The total preprocessor output is now ~7% smaller in bytes than initial
work, and ~10% in words, and builds ~7% faster (7.05s vs 7.55s before
on a 64-core EPYC). The part of the reasonable cleanup is now done.
However the complain_with_dbg function now appears in the caller's
stack as tail jumps are not always done, but we have plans for this.
Willy Tarreau [Tue, 11 Aug 2026 15:20:21 +0000 (17:20 +0200)]
CLEANUP: debug: make __BUG_ON() use abort_with_line()
ABORT_NOW() with no arg is strictly equivalent to abort_with_line(__LINE__)
so let's do it explicitly, not only it clarifies the code, but it
also reduces the size of the developed macros, which does count,
considering the number of BUG_ON() in the code.
Willy Tarreau [Tue, 11 Aug 2026 12:20:45 +0000 (14:20 +0200)]
MEDIUM: debug: do not dump a context-less backtrace in ABORT_NOW()
Originally before 2.4, ABORT_NOW() was used to instantly stop the program
with as little damage as possible in order to help debug it, keeping
registers intact.
This was modified in 2.4-dev6 by commit 5baf4fe31ad ("MEDIUM: debug:
now always print a backtrace on CRASH_NOW() and friends") because the
same macro was shared with BUG_ON() and we didn't have the backtrace.
But by doing this we lost the ability to debug the precise crash
location.
Later in 3.0, we added support for an extra contextual info with commit d417863828 ("MINOR: debug: support passing an optional message in
ABORT_NOW()"). This became even more fishy because at this point,
ABORT_NOW() called empty would only emit a backtrace, while when
passed an argument, it would emit "ABORT at <file>:<line>: <arg>",
losing the registers despite what the comment would say.
Now that we can type call places, it becomes possible to fix it again
so that ABORT_NOW() honors its two promises:
- no register mangling when called with no argument ;
- location + backtrace + message when called with an argument.
This patch simply conditions the call to ha_backtrace_to_stderr() to
the presence of an argument, without touching the abort_with_line()
call. It's done using an extra if to minimize the diff as it's only
temporary. For BUG_ON(), the trace calls are factored before the call
to an argument-less ABORT_NOW() since that one will not print the
message anymore.
Willy Tarreau [Tue, 11 Aug 2026 09:21:50 +0000 (11:21 +0200)]
MEDIUM: debug: add a new version of complain() that takes a debug_count struct
complain_with_dbg() takes a debug_count struct in argument from which
it retrieves everything it needs:
- details
- type
- file name
- line number
- description
- vaargs
This is done by making the common _complain() able to decode such elements
from a debug_count when presented. complain() passes a NULL there and uses
its msg, while complain_with_dbg() passes the debug_count and no msg. This
allows us to get rid of all the pre-made redundant messages and to save
70 kB of code. We're now around 136 kB under the initial size.
Willy Tarreau [Tue, 11 Aug 2026 09:13:14 +0000 (11:13 +0200)]
MINOR: debug: prepare complain() to be used in multiple ways
We'll intrument the function to do a bit more, but all call paths will
not be able to support this, so let's first turn it to a general one
as _complain() and have complain() just call it. Better not inline them,
we don't want to duplicate the footprint for something rarely used.
Willy Tarreau [Tue, 11 Aug 2026 08:04:15 +0000 (10:04 +0200)]
MINOR: debug: store the caller's details in debug_count
There's a 8-bit hole left in the struct and that's exactly what we'll
need to store the details. In fact we're only using 6 bits now and
they're a bit wasted, so we could even compact them to be merged with
the type later if needed.
Willy Tarreau [Tue, 11 Aug 2026 07:48:29 +0000 (09:48 +0200)]
MINOR: debug: rely on the __dbg_cnt_ counter for _BUG_ON_ONCE()
Since we already have a counter implemented when using DBG_COUNT(),
let's rely on this counter instead of declaring a local static one.
We always increment using a fetch_add() now, but regardless, this
reduces the code by 2 kB.
Willy Tarreau [Tue, 11 Aug 2026 07:43:13 +0000 (09:43 +0200)]
MINOR: debug: make a pair of __BUG_ON() for modern and obsolete linkers
The _BUG_ON() and _BUG_ON_ONCE() calls used to systematically call
__DBG_COUNT() first, then to call __BUG_ON(). Let's have a variant of
__BUG_ON() for each linker mode, that is merged with __DBG_COUNT().
The modern linker one now performs both the counting and processing
of the event at once.
For now the "once" aspect is still dependent on the expression-local
counter so that the code is exactly the same as before. The code size
didn't change.
Willy Tarreau [Thu, 13 Aug 2026 14:51:11 +0000 (16:51 +0200)]
CLEANUP: debug: remove the unneeded do { } while (0) on __BUG_ON()
The macro is exclusively called by the _BUG_ON*() macros so it cannot
appear anywhere else, no need to clutter the emitted syntax for nothing,
seeing the output of the preprocessor on these parts is totally scary
already.
Willy Tarreau [Tue, 11 Aug 2026 07:32:02 +0000 (09:32 +0200)]
MINOR: debug: avoid the break in __BUG_ON()
Since the "break" statement irritates gcc, let's resort to the
opposite expression involving a compount expression to declare the
static counter. It seems to be doing the job well, and we're back
to the previous size (-63 kB from original).
Willy Tarreau [Mon, 10 Aug 2026 18:07:01 +0000 (20:07 +0200)]
MINOR: debug: unify __BUG_ON() and __BUG_ON_ONCE()
We can do that thanks to the debug type. The line counter moved into an
if () block conditioned by the type so it continues to emit nothing for
regular bugs.
Surprizingly, the code grew by 1 kB. It's apparently caused by having
an impossible "if" statement containing a "break". Splitting the macro
in two to put the BUG_ON_ONCE() callers aside showed that having only
"if (0) break;" in the block was sufficient to make gcc deoptimize it
and result in this same growth. This was observed with 9.5. Let's
consider this a temporary limitation and keep it this way.
Willy Tarreau [Mon, 10 Aug 2026 16:44:41 +0000 (18:44 +0200)]
MEDIUM: debug: make the complain() function print the suffix
The suffix, such as "please contact the developer" that is printed on
each warning, with its optional variant "not crashing ..." can be entirely
computed by complain() based on the flags description. This way we can drop
that from the macros and further shrink the messages.
Willy Tarreau [Mon, 10 Aug 2026 17:27:01 +0000 (19:27 +0200)]
MEDIUM: debug: use \x1e (RS) to delimit the condition from vaargs in complain()
This is the same principle as for DBG_COUNT() but applied to complain().
For now it doesn't bring any particular benefit but we needed to know where
the end of the string is in order to continue to turn other fields to
variables.
Willy Tarreau [Mon, 10 Aug 2026 17:02:28 +0000 (19:02 +0200)]
MEDIUM: debug: use \x1e (RS) to delimit the condition from vaargs in DBG_COUNT()
The principle here is to reduce the reliance on multiple pointers in the
debug_count struct while still keeping a delimitation between records.
For this, we use the ASCII record separator character (\x1e or RS) that
is not present in the rest of our messages and has no reason to land there
by accident.
This slightly reduces the code size (2kB) but the purpose is to prepare
a next step where these definitions will benefit the BUG_ON*() macros.
Willy Tarreau [Mon, 10 Aug 2026 15:49:39 +0000 (17:49 +0200)]
MEDIUM: debug: make the complain() function print the prefix
The prefix ("ABORT at", "FATAL:", "WARNING:") as well as the second level
("bug condition", ...) are entirely determined by the details bit field,
so there is no reason to have to build this string in the caller, instead
we can let complain() build it entirely from the details into the local
iovec, so let's do that.
The prefix is now dropped from the whole call chain as it's no longer
useful. This further reduces the code size by 17.5 kB.
Willy Tarreau [Mon, 10 Aug 2026 16:28:38 +0000 (18:28 +0200)]
MINOR: debug: make the complain() function start to complete the message
The function will be extended to automatically prepend/append some info.
For now all it does is to always prepend/append the "\n" that each message
has at the beggining and at the end.
We're doing this into a locally allocated iovec and emit it at once using
writev(). This will be easy to extend to pass extra info. The \n alone
removed 1.5 kB of code.
Willy Tarreau [Mon, 10 Aug 2026 15:32:57 +0000 (17:32 +0200)]
CLEANUP: debug: remove the now unused counter argument to complain()
Let's drop it before someone has the idea to use it again.
We take this opportunity for swapping complain()'s arguments which were
really poorly ordered since we're really passing a context first, so the
calls read better this way, particularly since we plan to add more.
This drops another 4 kB of code due to the number of calls.
Willy Tarreau [Mon, 10 Aug 2026 15:01:13 +0000 (17:01 +0200)]
MINOR: debug: only emit and count warnings when explicitly requested
Previously complain() would set TAINTED_WARN was set even for a
check_if() because there was no way to distinguish a warn from a check
and a warn was systematically accounted for when the type was not a
bug. Likewise, BUG_ON() would dump a trace whenever a non-fatal event
happened, which is not the goal either as it prevents from counting
events without logging them (e.g. COUNT_IF). Now that we have explicit
flags for each type, let's consider them when choosing what to emit.
Note that for now complain() will still log the "msg" line if used
without FATL nor WARN though.
Willy Tarreau [Mon, 10 Aug 2026 14:16:34 +0000 (16:16 +0200)]
MINOR: debug: use bit fields for the debug type and fatality
Till now the BUG_ON() series of macros has been relying on magic values
0,1,2, and 3 passed down the chain to decide whether to log a bug or a
warning, and whether to crash or just warn. But this doesn't capture the
whole usage and is causing difficulties because we also have type "check",
and these values are passed to variables historically named "crash" or
"taint" suggesting booleans while bit-exact checks have to be done.
Let's replace them with a combination that separately indicates:
- the type (check, warning, bug, abort), one bit per type, expected
to be exclusive
- type fatality (warning or fatal), one bit per level, must be
exclusive as well
This allows us to easily replace the checks and map the numbers to their
equivalent definitions. The macros are called DBG_DET_* (debug details),
and the "crash" and "taint" fields have been renamed "details" as well.
For now this is the exact equivalent of what we previously had.
Willy Tarreau [Wed, 5 Aug 2026 14:22:40 +0000 (16:22 +0200)]
MINOR: debug: move printing of the hint to ha_backtrace_to_stderr()
The hint suggesting the user to contact developers with a core is only
emitted along with a backtrace sent to stderr, and it's currently
present as a macro in many call places. Let's move it to the function
and pass an argument to ask for it to be emitted (i.e. only upon crash
since a warning doesn't produce a core).
Willy Tarreau [Mon, 10 Aug 2026 14:31:14 +0000 (16:31 +0200)]
DEBUG: deduplicate __ABORT_NOW() between DEBUG_USE_ABORT and ha_crash_now()
The __ABORT_NOW() macro is defined for the two modes with a single,
insignificant difference, which is how to crash. Given that there's
already an #ifdef for the abort case, better make that macro point
to ha_crash_now() for the default case and eliminate one of the
definitions. This code is sufficiently difficult to read to avoid this.
Willy Tarreau [Mon, 10 Aug 2026 14:29:44 +0000 (16:29 +0200)]
DEBUG: cli: add a "debug dev abort" debugging command
We had "debug dev bug/check/warn" to test the various BUG_ON() macros
but we didn't have a way to test ABORT_NOW(). So let's introduce
"debug dev abort" for this.
Willy Tarreau [Mon, 10 Aug 2026 14:50:39 +0000 (16:50 +0200)]
MINOR: debug: check the match count in __BUG_ON_ONCE() and not in complain()
The fix in commit 7a78d6c600 ("BUG/MINOR: debug: only dump the trace once
in __BUG_ON_ONCE()") warned that it's not strictly atomic, but we could
do better and take this opportunity for cleaning the code: use a fetch-add
in __BUG_ON_ONCE() and pass NULL to complain(), which thus no longer has
any call place requiring it to check a counter. This only very slightly
inflates the code (~138B) since each BUG_ON_ONCE() now has to load 1,
xadd(), check the return value instead of leaving it to complain(), but
this is totally marginal compared to the benefits.
This can even be backported where the fix above is backported if needed.
Willy Tarreau [Wed, 12 Aug 2026 14:04:43 +0000 (16:04 +0200)]
CLEANUP: debug: reorder definitions in bug.h
This file has become totally unreadable because it's littered with lots
of #if, and mixes unconditional and conditional declarations that have
been accumulating over time. Let's first reorganize it so that:
- externs are defined first
- isolated macros are defined second
- enums/structs are defined third, optionally with their macros and
inline functions if unconditional.
- then the conditional stuff.
A few types were made permanent as there's no point hiding them behind
ifdefs, except complicating a developer's life in another file.
Now most common stuff is quickly found and the more involved stuff remains
at the end.
Willy Tarreau [Tue, 11 Aug 2026 06:05:35 +0000 (08:05 +0200)]
CLEANUP: debug: move ha_backtrace_to_stderr() declaration to bug.h
The DUMP_TRACE() macro was made just to manually reference the external
ha_backtrace_to_stderr() function that is defined in debug.h, but this
makes the code even harder to follow. In fact, the current rule that
bug.h contains all mandatory declarations and that debug.h contains the
optional ones is not respected here, so let's just move the declaration
there and get rid of the macro.
Willy Tarreau [Tue, 11 Aug 2026 13:26:52 +0000 (15:26 +0200)]
DEBUG: cli: unstatify cli_process_cmdline()
There's no point keeping this function static, and because of this it's
never resolved in backtraces despite appearing quite often since it's
located between the io_handler and various parse functions that may
possiblly trigger issues. So let's remove the static modifier on it.
REGTESTS: checks: skip tcp-check-client-hello.vtc under FIPS mode
This test hangs and gets killed after a 10s timeout when run against
an SSL library running in FIPS mode (e.g. AWS-LC-FIPS): backend be1's
check server line uses "curves X25519" to exercise X25519 key-share
routing (group 0x001d), which is rejected at config-parse time by
HAProxy's own FIPS compliance checks (src/fips.c) since X25519 is not
a NIST P-curve. The "h1" haproxy process then fails to start, and
vtest waits on it until its startup timeout kills it with SIGKILL
instead of reporting a clean startup failure.
Skip the test using the fips_mode() config condition predicate.
Both reg-tests fail when run against an SSL library running in FIPS
mode (e.g. AWS-LC-FIPS), for two distinct reasons:
- tls12_ssl_crt-list_filters.vtc configures the "kRSA" (non-ECDHE)
TLSv1.2 cipher suite, which is rejected at config-parse time by
HAProxy's own FIPS compliance checks (src/fips.c).
- ssl_generate_certificate.vtc's P-384 check observes the default
curve/group (e.g. P-256) instead of the configured secp384r1: the
"ecdhe" bind keyword does not appear to restrict the negotiated
curve/group in this mode, reproduced independently of the TLS
version forced by the client.
Skip both using the newly introduced fips_mode() config condition
predicate rather than disabling them outright, so they keep running
everywhere else.
MINOR: ssl: report FIPS mode in -vv for OpenSSL >= 3.0 too
The "SSL library FIPS mode" line in "haproxy -vv" was only computed
for SSL libraries implementing the legacy FIPS_mode() API (OpenSSL
1.0.x/1.1.x and compatible libraries such as AWS-LC), and silently
omitted for OpenSSL 3.0 and above.
Use the openssl_fips_mode() helper introduced for the fips_mode()
config condition predicate instead of calling FIPS_mode() directly:
it also covers OpenSSL >= 3.0 via
EVP_default_properties_is_fips_enabled(), and reports "no" rather
than omitting the line entirely for any SSL library supporting
neither API. The line is now unconditionally printed.
Add a new "fips_mode()" predicate usable in .if/.elif configuration
conditional blocks and with the "-cc" command line option. It
evaluates to true when the loaded SSL library is currently running in
FIPS mode.
The check relies on a new openssl_fips_mode() helper in tools.c,
following the same pattern as openssl_compare_current_version() and
awslc_compare_current_api(). Two APIs are used depending on the SSL
library:
- FIPS_mode(), implemented by OpenSSL 1.0.x/1.1.x (including
FIPS-validated builds) and by compatible libraries such as AWS-LC.
- EVP_default_properties_is_fips_enabled(), for OpenSSL 3.0 and
above, where FIPS_mode() was removed in favor of a provider-based
FIPS model. It reports whether the default library context
currently resolves algorithm fetches to the FIPS provider, which
is the closest 3.x equivalent.
The predicate is a no-op (always false) with any other SSL library, or
when built without SSL support.
This lets configurations, and reg-tests in particular, detect and
adapt to (or skip) TLS constructs that are rejected by HAProxy's own
FIPS compliance checks (src/fips.c) when FIPS mode is active, e.g.
non-ECDHE TLS 1.2 cipher suites.
Olivier Houchard [Thu, 13 Aug 2026 13:57:56 +0000 (15:57 +0200)]
BUG/MEDIUM: bwlim: fix a stick-table entry leak in shared mode
In shared mode, bwlim_set_limit() stores the stick-table entry returned
by stktable_get_entry(), which takes a reference, into st->ts without
releasing any entry already stored there. When set-bandwidth-limit is
executed more than once for the same stream and shared filter, the
intermediate reference is leaked: bwlim_detach() only releases the last
one. The entry then stays pinned (ref_cnt > 0) and can never be purged,
and with distinct keys the table fills up until the features depending
on it are denied service.
Release the previously held entry before overwriting st->ts.
This was introduced in 2.7 by commit 2b6777021 ("MEDIUM: bwlim: Add
support of bandwith limitation at the stream level") and must be
backported to all stable branches.
Many thanks to Red Hat and AISLE Research for reporting the issue and
providing a fix.
Willy Tarreau [Thu, 13 Aug 2026 13:03:26 +0000 (15:03 +0200)]
MINOR: sample: add new converter has_ctl() to detect control characters
Control characters, as defined by RFC5234, are 0x00 to 0x1F and 0x7F.
They're not easy to insert in a config by definition and difficult to
match. Let's add a sample converter which takes a binary input and
returns a boolean indicating if any such character is found within a
possibly configurable class.
By default with no argument, it checks the range above except TAB (0x9)
which is common. With argument "any", it checks them all. With argument
"http", it only checks the strictly forbidden HTTP ones (CR, LF, NUL)
in headers. Otherwise it takes a mask made of the bits corresponding
to each character, with bit 32 corresponding to character 0x7F.
It can be used to detect anomalies, e.g. by logging or dropping when
any such character is found.
Willy Tarreau [Thu, 13 Aug 2026 09:49:26 +0000 (11:49 +0200)]
DOC: security: clarify that only up-to-date versions may get securty reports
With the increasing rate of low-effort, AI-driven security reports,
we're seeing an increasing level of noise. Let's first clarify what's
usually obvious to developers but not necessarily to bug reporters,
which is that vulnerability reports must exclusively apply to latest
version of a branch. The goal here is to reduce the amount of time
wasted analyzing an issue to finally respond "already fixed 3 months
ago".
Let's also add the link to the bugs page to ease extra checks on the
reporter's side when they cannot re-run the scan (which usually is
the case when time slots are granted on software scanners and they're
run against an outdated version).
This must be backported where the security doc is already present.
Mani Goyal [Wed, 12 Aug 2026 06:56:22 +0000 (12:26 +0530)]
BUG/MEDIUM: http: fix authority parsing for absolute-form URI with empty path
http_parse_authority() only stopped scanning at '/', not '?' or '#'.
For an absolute-form request-target with no path but a query string
(e.g. "http://host?token=..."), the authority scan ran to the end of
the URI and swallowed the query string into the authority. This
caused http_scheme_based_normalize() to see an empty path and append
'/' after the query string instead of between the host and the
query, corrupting the request on the wire. The same corruption
happens with a literal '#' in the request-target when HTTP
violations are tolerated (option accept-unsafe-violations-in-http-
request), since it is not rejected by the request-line parser in
that mode either.
Per RFC 3986 section 3.2, authority terminates at '/', '?', or '#',
or at the end of the URI, so also stop at these two delimiters.
Add cases to h1_host_normalization.vtc covering an empty path with a
query string, with and without a port needing normalization. Add a
new h1_authority_fragment_char.vtc covering the '#' terminator
specifically, since it requires accept-unsafe-violations-in-http-
request to reach the parser at all and doesn't belong in the shared
frontend used by the other host-normalization cases.
This should be backported to all stable versions.
Should fix issue #3460.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BUG/MINOR: server: fix off-by-one error when parsing and copying source port range
When allocating a source port range, port_range_alloc_range() allocates
a ring structure of size n + 1 to accommodate a sentinel slot for the
lock-free ring buffer.
However, both srv_parse_source() and srv_conn_src_sport_range_cpy()
were incorrectly using range->size directly, filling and copying the
sentinel slot as if it were a valid port. This ->size port_range struct
field should never be used.
This off-by-one error caused an extra port to be populated. When copying a
configuration (e.g. via default-server or server-template), this extra
slot became allocatable, allowing connections to bind beyond the configured
port range (e.g. binding port 5002 when 5000-5001 was set). Furthermore, if
the range reached port 65535, the extra port wrapped to 0, producing
CO_ER_PORT_RANGE connection failures.
Fix this by introducing port_range_count() to cleanly return the number of
usable ports (range->size - 1), and use it in both srv_parse_source()
and srv_conn_src_sport_range_cpy().
Many thanks to Red Hat and AISLE Research for reporting this.
BUG/MINOR: h3: fix potential NULL pointer dereference in _h3_trace_header()
This bug can be triggered only when the h3 traces are enabled.
It has been reported by coverity in GH #3469 where qcc was checked for NULL
earlier in _h3_trace_header(), then dereferenced without a check via qcc->conn
during the next TRACE_PRINTF_LOC() call.
BUG/MINOR: server: fix memory leak on "default-server" parsing failures
This is a fix for very minor bug which may be triggerd only at parsing time
upon parsing failures (allocation failure, or default server name mismatch).
When parsing a named "default-server" instance in _srv_parse_init(), <name> is
allocated via strdup(). If a name mismatch occurs or if srv_alloc() fails,
the function jumps to the <out> label without freeing this string.
Fix this by initializing <name> to NULL at the function start and calling
free(name) in the <out> cleanup block.
Willy Tarreau [Thu, 6 Aug 2026 07:45:26 +0000 (09:45 +0200)]
BUG/MINOR: quic: avoid a division by zero in the BBR pacing interval
bbr_pacing_inter() divides by the connection's pacing rate without
checking it. That rate is recomputed as a truncated integer product of the
estimated bandwidth by the pacing gain, and once full bandwidth has been
reached the new value is stored unconditionally, even when it truncated
down to zero. So a QUIC peer that acknowledges slowly and sparsely enough
to drive the bandwidth estimate down to a single byte per second, then
lets the connection leave Startup, makes the next pacing computation
divide by zero.
The resulting SIGFPE is not caught, so it takes down the whole worker
along with every connection it was serving, which any remote QUIC client
controls the conditions of.
Let's floor the divisor at 1, as the default pacing helper effectively
does.
BUG/MINOR: ssl: reject an embedded NUL in the full-DN ssl_*_dn() fetches
ssl_sock_get_dn_oneline() has the same flaw fixed in ssl_sock_get_dn_entry()
by the previous commit: it copies each DN component's ASN.1-declared bytes
verbatim via memcpy(), keeping an embedded NUL and whatever follows it. It
is used to build the full one-line DN returned by ssl_c_s_dn/ssl_c_i_dn/
ssl_r_dn when no field argument is given, so the same tree-vs-list ACL
match inconsistency applies to it.
Apply the same fix: reject the whole DN (return 0) if any entry has a NUL
followed by a non-NUL byte, and otherwise keep a single trailing NUL and
skip any extra ones.
This should be backported to every stable branch, alongside the previous
commit.
BUG/MINOR: ssl: reject an embedded NUL in the ssl_*_dn(entry) fetches
ssl_sock_get_dn_entry() copied the ASN.1-declared bytes of a DN entry
verbatim, keeping the exact length. pat_match_str()'s tree branch
(used by the default, exact-match "-m str" ACLs) then looks the value
up with ebst_lookup(), which treats it as a NUL-terminated C string.
A certificate with e.g. CN = "admin\0.attacker-owned.example" is
therefore matched and authorized as "admin": the producer keeps the
full value, but the ebtree consumer silently truncates at the first
NUL. ACLs using "-i" are unaffected since they take the length-correct
list branch instead of the tree.
Reject such a value in the fetch itself, before it reaches the
pattern-matching layer: a NUL followed by any further non-NUL byte
now makes ssl_sock_get_dn_entry() fail as if the entry did not exist,
so ssl_c_s_dn(CN)/ssl_c_i_dn(CN)/ssl_r_dn(CN) and any map/ACL relying
on it can't be bypassed this way. Trailing NUL byte(s) with nothing
after them aren't a truncation risk, so a single one is kept in the
returned length instead of the whole ASN.1 length, and any extra ones
are skipped.
The impact is low since it would need to be signed by the CA anyway.
IMPORT: slz: prevent build compilation error due to unused __slz_make_crc_table()
In __slz_common_initialize(), __slz_make_crc_table() was not
systematically called (ie: when __ARM_FEATURE_CRC32 is defined the
crc32_fast[] table is not used). However since the function is defined
as static, AND its content is already guarded by the same
__ARM_FEATURE_CRC32, we actually need to call always call the function,
else we could face a compilation warning on some systems such as:
scientiamobile [Tue, 11 Aug 2026 10:15:39 +0000 (12:15 +0200)]
BUG/MINOR: wurfl: fix memory leak of information list and patch strings at deinit
The strings duplicated with strdup() for each "wurfl-information-list" token
(wi->data.name) and each "wurfl-patch-file" path (wp->patch_file_path) were
never freed: ha_wurfl_deinit() only freed the list nodes, not the strings they
own. A simple config check on a configuration using these keywords is enough to
leak them, as reported by ASAN.
Free wi->data.name and wp->patch_file_path before freeing their list nodes.
This bug has been present since the module was introduced in commit d0027ed5b
("MEDIUM: wurfl: add Scientiamobile WURFL device detection module"). It should
be backported to all stable versions.
This fixes issue #2084. Reported-by: Ilya Shipitsin <chipitsine@gmail.com>
BUG/MEDIUM: cache: do not release an entry under the cache read lock
In http_action_req_cache_use() the cache lock is taken in read mode rather
than write mode in order to keep decent performance. But the secondary key
lookup releases the primary entry from there with needs_locking = 0, which
means "the caller already holds the write lock", because dropping the last
reference removes the entry from the tree. A removal could therefore run
while other threads were walking the same tree under their own read lock.
Release the entry after the read-locked section instead, where taking the
write lock for the removal is allowed.
The row is handed back only once the reference has been dropped, and no
longer before: a row that returns to the avail list while a reference is
still held on its entry can be recycled by another thread, and the release
would then be applied to blocks that hold a response body. This is also why
the reattach cannot simply be left where it was.
This is present at least as far back as 3.0 and should be backported to all
stable branches.
BUG/MINOR: cache: do not retain imcomplete or stripped secondary entry
Skip retaining secondary entries altogether if its row was recycled
while we were looking it up, which CACHE_EF_COMPLETE having been cleared
tells us. Such an entry was already treated as a miss, only further down
and after having been detached.
BUG/MEDIUM: cache: retain the primary or secondary entry only when detaching its row
http_action_req_cache_use() retains the entry returned by the lookup, then
takes the shctx lock to detach its row. Until that detach the row is still
in the avail list, and shctx_row_reserve_hot() recycles a row under the
shctx lock alone, without ever taking the cache lock, so the cache read
lock does not protect a retained entry. In that window another thread can
recycle the row: the entry is queued on the cleanup list and its blocks are
refilled with a response body, after which our detach works from a
block_count and a last_reserved that no longer describe that row, splices
the wrong blocks out of the avail list and takes a reference on blocks
owned by another row. A block then sits both in the avail list and in a
live row, and the next thread reserving it reads a body as a cache_entry.
Retain the entry under the same shctx lock that detaches its row, and
only once it is known to be usable. A row recycled in the meantime is no
longer considered complete, which cache_free_blocks() clears under that
lock, so the existing test already rejects it. The entry is still
readable at that point because recycled blocks are only written once
shctx_row_reserve_hot() has returned, which cannot happen before
cache_reserve_finish() has taken the write lock on the tree our read
lock holds.
The other path then holds no reference, so it has nothing to release.
IMPORT: slz/uslz: defer the output bookkeeping to a checkpoint
Every emitted byte went through _PUT_UPDT(), which incremented five separate
counters by the same length (dec_bsize, dec_total, index, crc_flush and
distance_avail) and tested three limits, plus a fourth test on dec_bsize
before each symbol and a rollover test per loop iteration. For a literal
that is about ten operations and four branches around a single store, and
crc_flush lives in the state structure, so it was a read-modify-write
through memory which could never stay in a register.
Those counters are all the same position expressed in different units,
so we now only maintain the output pointer <out> and compare it against
a precomputed limit <out_lim>, the closest of the three points where
something actually has to be done:
- the end of the ring, where <out> wraps back to <out_base> ;
- the end of the current checksum batch ;
- the point where the decoded block fills the caller's buffer.
When <out> reaches <out_lim> we reach a single "checkpoint" which accounts
for the bytes produced since the previous one, checksums them while they
are still hot in the cache, wraps the ring, reports a full buffer if
needed, recomputes the limit and jumps back to whichever of the three
emitting sites called it. Emitting a literal is now a store and a single
pointer increment, and the counters move at most once every CRC_BLOCK
bytes. The emitting loops have to compare against something to know when
to stop anyway, so the test on <out_lim> is not an added cost.
This comes with multiple benefits:
- distance_avail is no longer needed. It was min(dec_total, out_max)
and was only ever compared against a distance, which never exceeds
32kB while the ring is at least that large, so "have we produced at
least <distance> bytes" is exactly "has the ring wrapped, or is the
distance within the current position". A <wrapped> flag gives the
first half and the match copy already computes the second, so the
check became free.
- crc_flush was removed from the state. Rather than carrying pending
bytes across calls, the return paths now flush the checksum, which
continues to guarantee that on entry the ring position is exactly
dec_total % out_max with nothing pending. That is what makes the
whole thing work, and it also removes by construction the overflow
that this field had to take care of.
- the match copy is now driven by <out_lim>: since the destination
can no longer wrap in the middle of a chunk, the four interleaved
copy phases collapse into two cases, the match starting in the
upper part of the ring or below the current position. A match no
longer has to fit entirely in the remaining space either, it is
emitted in as many chunks as needed, so USLZ_DECODE_E_OUT_BUFFER
is now unreachable.
- the stored block copy becomes a plain loop bounded by the input,
the remaining block length and the limit.
- the rollover check on dec_total moves to the checkpoint, the only
place where the value changes now.
Measured with tests/codepad.sh, decompressing silesia in slz or in
gzip forms now shows:
before after
silesia.tslz 0.8762 s 0.7837 s -10.6 %
silesia.tgz 1.5175 s 1.3875 s -8.6 %
And as a bonus, the code is now smaller (measured on x86_64):
IMPORT: slz/uslz: inline the bit reader and the huffman decoders
gethuff(), gethuff_fixed() and bit_accumulate() were all purposely marked
noinline due to tests showing a significant performance decrease and code
increase by inlining them. That's annoying because most of the arguments
they need can easily be optimized once the compiler has a full view of
them, and it's quite visible with perf top when decompressing an slz stream
that 30% of the CPU is spent in gethuff_fixed() and bit_accumulate(), and
that when decompressing a gzip stream, it's 62% in gethuff() alone!
It turns out that it's only combinations of 1 or 2 of them inlined that
ruins the performance, but inlining the 3 at once instead shrinks the
code and boosts the performance by letting the compiler keep all these
local variables in registers. As a proof, the code is now ~352 bytes
smaller, and 13 to 17% faster (respectively for slz and gzip streams).
That's one example of situations where individual changes bring nothing
good.
IMPORT: slz/uslz: add a method to impose an envelope format (gzip or zlib)
This is actually a way to refuse non-matching formats and not letting the
decoder auto-detect a format. This is done using uslz_init_fmt() with a
4th argument instead of uslz_init():
- SLZ_FMT_DEFLATE skips detection entirely, since the first bits of the
stream are already the first block header (deflate has no envelpoe).
- SLZ_FMT_GZIP and SLZ_FMT_ZLIB parse their envelope as before, but the
stream is now rejected with E_CORRUPT if it does not carry the
announced one, instead of falling back to raw deflate and returning
garbage. That might matter when the format comes from the outside, an
HTTP Content- Encoding for instance, where accepting a different
envelope is wrong (though would likely have to be accepted anyway).
The main purpose is in fact to use this in tests to validate specific
envelopes, something that is currently not possible (i.e. if zenc lies,
zdec silently adapts).
IMPORT: slz/uslz: decode all the members of a multi-member gzip stream
A gzip file is a series of members (rfc1952), which is what "gzip -c a b",
"cat a.gz b.gz" and most log rotators produce. uslz stopped after the first
one and returned USLZ_DECODE_SUCCESS, so the caller silently got truncated
content with no way to notice:
This is not commonly used with our targetted use cases but can sometimes
be seen in incremental backups for example where extra inputs will be
ignored.
The required change is not that big but is not obvious:
- First, the gzip trailer is 8 bytes, crc32 followed by isize, and only
the crc32 was consumed. The four isize bytes were left in the stream,
so nothing could have recognised the next member's magic behind them.
Both halves are now accumulated before anything is compared, which
also makes the trailer read resumable without having to remember how
far into it we got; isize is consumed but not verified.
- Second, a new function, uslz_next_member() detects a following member
and resets the per-member state (checksum, flags, state machine, bit
accumulator) while preserving the output ring, the total decoded size
and the drain offset, so that the members' contents are simply
concatenated. It is called both right after a member completes, so
that a single call decodes as many members as its input holds, and at
the start of a call, so that a member boundary falling between two
calls works too.
The two magic bytes may be split across calls, and the header buffer
cannot hold them in the meantime because it shares storage with the
distance table which the member just decoded has overwritten. The bit
accumulator isn't used at that point and survives across calls, so it
is reused here to store the previous bytes and the confirmed magic is
then passed to the format detection, which already knows how to
accumulate the rest of a header across calls.
- Third, when a member ends exactly at the end of the input, there is no
way to tell whether another one follows without more data. Success is
reported, which is what a caller with nothing left to send needs, and
the check is retried on the next call for a caller which has more.
This is now stated in uslz_decode()'s documentation: success means
complete as far as the data provided goes, and a caller with input
left must call again anyway. Anything after the last member which is
not a gzip magic is ignored as trailing garbage, as gzip(1) does.
With all this done, concatenating two silesia archives and passing them
to zdec properly now reports twice the uncompressed size.
This was the last failure of tests/uslztest.sh which now shows 1514/1514.
IMPORT: slz/uslz: verify the trailer checksum before reporting completion
uslz_decode_block() sets USLZ_FL_COMPLETE at the top of its completion
path, before reading and comparing the trailer checksum. If the trailer
bytes were not available yet, we jump to out_of_data, and uslz_decode()
then sees a complete stream in its drain path and jumps to end,
returning USLZ_DECODE_SUCCESS without ever calling uslz_decode_block()
again. Thus the checksum is never verified at all in this case, meaning
that under some scheduling circumstances, an invalid stream could appear
as valid.
On top of that, nothing ever assigns USLZ_ST_CHECK_CKSUM, so even if the
decoder had been called again, the state machine could not have resumed
in the middle of the trailer: it would have gone back to decoding symbols
and interpreted the trailer as deflate data.
The result was that a stream with a corrupted checksum was accepted,
silently, depending only on where the caller happened to cut its input.
On a 319-byte zlib stream the corruption went unnoticed at chunk sizes
1 to 7, 9, 15, 21, 35, 45 and 53, and on a 331-byte gzip stream with a
corrupted crc32 at 12, 13, 17, 18, 19, 25, 27, 36 and 54. (reminder,
deflate has no checksum).
This patch addresses this problem this way:
- USLZ_ST_CHECK_CKSUM is set before reading the trailer, so that an
out of data return really can resume there.
- crc_flush is cleared after the final uslz_update_crc(), so as not
to checksum the same bytes a second time in case of resume.
- USLZ_FL_COMPLETE is set only *after* the comparison succeeded.
Now, trying to decode a corrupted trailer for either a zlib or gzip
stream properly reports a corruption (tested with 80 positions for each).
IMPORT: slz/uslz: widen crc_flush, it overflowed on large stored blocks
crc_flush counts the bytes decoded but not checksummed yet. It was
declared as a (signed) short, while a single stored (uncompressed) block
copy can add up to a whole output buffer in one call, making it overflow:
crc_flush=-32768 after +=32768 (index=32768)
That happens with any incompressible payload, which the encoders emit as
stored blocks, as soon as the input is fed in chunks large enough for the
copy to advance by more than 32767 bytes at once. The subsequent call to
uslz_update_crc() then gets a negative length. The checksum functions
happen not to touch memory in that case (their loops simply do not run),
so the visible symptom is a wrong checksum and a valid stream rejected
with USLZ_DECODE_E_BAD_CRC -- but the invariant crc_flush <= index is
broken from then on, and (out_base + index - crc_flush) can point before
the output buffer, which would read out of bounds.
Here we perform two changes:
- crc_flush becomes an int. It has to be larger than the output
buffer, not smaller.
- the CRC_BLOCK batch flush becomes a while loop instead of an if.
It only ever flushed one CRC_BLOCK per emitted chunk, so a large
copy left the rest pending and defeated the point of batching the
checksum in blocks small enough to stay in L1. With the loop,
crc_flush is back below CRC_BLOCK after every emission, which also
bounds it tightly.
It's easy to reproduce with a 40kB incompressible file (e.g. urandom):
IMPORT: slz/uslz: make the gzip FEXTRA field resumable
The FEXTRA optional header field is a 2-byte little endian length followed
by that many bytes to skip, and either part can be split across calls. The
code had a fast path and a slow path for each of the two parts, and used
buf_len=0 to mean "nothing pending" but that is also true right after the
fast path has consumed the length and found the payload incomplete.
Resuming from that point took the fast path again and read the length a
second time, from what were in fact the first two payload bytes. With the
payload being "ABCDEF", XLEN became 0x4241 = 16961 instead of 6, so the
decoder skipped 16963 bytes of deflate data and the stream was lost.
This could be reproduced with a gzip stream carrying FEXTRA, FNAME,
FCOMMENT and FHCRC: it failed for input chunk size from 2 to 8 and 12
to 17, i.e. whenever a call boundary happened to fall inside the FEXTRA
field.
Let's replace the four paths by one: always stash XLEN in the header
buffer and use buf_len as the count of bytes of the whole field
consumed so far, so that below 2 we are reading the length and above
we are skipping the payload. That is resumable at any byte and needs
no special case, at the cost of skipping the payload one byte at a
time, which is fine for a field that is rare and usually a few bytes
long.
tests/uslztest.sh goes from 1438/1514 to 1488/1514.
IMPORT: slz/uslz: don't lose the bytes consumed by the header fast paths
The format autodetection in uslz_decode() has a fast path for the common
case where the first call carries enough data to parse the header directly
from the caller's buffer, and a slow path that accumulates the header into
hdr_detect.buf across calls. The fast path consumes bytes from the caller's
buffer *before* it knows whether it will be able to complete, and it had no
way to give them back, so three things went wrong:
1) Raw deflate (rfc1951) streams were losing their first two bytes. The
fast path consumes two bytes to sniff the magic, and the fallback that
feeds them back to the block decoder is guarded by "else if (buf_len)",
which is false on exactly that path. Every raw stream fed more than 2
bytes at a time therefore failed with E_CORRUPT (or E_INVALID_BLOCK_CODE,
depending on what the two lost bytes happened to decode as), i.e. raw
deflate was simply unusable:
./zdec < raw.deflate -> error (14)
This is fixed by rewinding in_ptr to the start of the header, which
also lets the fast path skip the local copy the accumulating path
needs.
2) A gzip stream whose first call carried 3 to 9 bytes was mis-parsed.
That is more than 2, so the fast path consumed the magic, but less
than 10, so the 10-byte header could not be parsed, and it fell back
to need_more_header, which starts filling hdr_detect.buf at buf_len=0,
dropping the two bytes already consumed. The next call then parsed the
header two bytes too far. This is fixed by rewinding before the
fallback.
3) The tests on the remaining input used <compressed_size>, which is only
decremented by the fast paths and never by the accumulating ones, so it
overstates what is left in the caller's buffer as soon as the two are
mixed. "if (compressed_size >= 2)" and "if (compressed_size >= xlen)"
could therefore both succeed with fewer bytes actually available, read
the FEXTRA length or payload **past in_top** and advance in_ptr beyond
it. Fixed by testing state->in_top - state->in_ptr, which is exact on
every path; <compressed_size> is no longer modified.
tests/uslztest.sh goes from 1108/1514 to 1438/1514, and raw, gzip, zlib
and slz streams now decode correctly at every input chunk size from 1 to
24 and beyond, for every supported ring size. The gzip variant carrying
FEXTRA is still broken at some chunk sizes, this is for a future fix.
IMPORT: slz/uslz: don't reset the drain offset when nothing was decoded
The drain code in uslz_decode() resets dec_bofs to zero when the amount
of data it can report is zero, in order to handle the case where the
previous drain stopped exactly on the end of the ring and the pending
block restarts at its beginning.
But this test also matches when there is simply nothing pending at all,
which happens on every USLZ_DECODE_OUT_OF_DATA return that could not
decode a single byte, i.e. very often when the caller feeds small input
chunks. In that case dec_bofs was reset while the pending data was in
fact located further in the ring, and the next drain handed the caller a
pointer to the wrong place, silently returning stale bytes.
The decoded stream was correct (the checksum matched), only the pointer
reported to the caller was wrong, which made this hard to notice: zdec
uses 8kB input chunks and almost always has something to drain.
Fix this by only resetting dec_bofs when there really is pending data.
With the new tests/uslztest.sh matrix this moves the number of passing
combinations from 642/1260 to 848/1260, the remaining failures being
pre-existing issues in the raw deflate and zlib trailer handling and in
the gzip header parsing with very small input chunks.
IMPORT: slz/uslz: fix incorrect sign extension in shift when reading the adler32 trailer
The zlib trailer is assembled byte by byte into the 64-bit bit accumulator:
bit_accum |= in_ptr[0] << (24 - num_bits);
in_ptr[0] is an unsigned char, that is unfortunately promoted to signed
int when shifted left, thus introducing a sign bit in upper bits if bit 7
was set with num_bits=0, that gets sign-extended to 64-bit in the
accumulator, resulting in a wrong checksum (and the stream is rejected
with USLZ_DECODE_E_BAD_CRC). Let's just cast it to uint32_t before shifting
to fix this. Note that other similar places already had the cast, this one
was just overlooked.
IMPORT: slz: clarify that the size promise applies to the stream, not to a call
The output size guarantee of slz_rfc1951_encode() reads as if it applied
to every call, but up to 31 bits are retained in the queue from one call
to the next (on 64-bit systems), so a call may emit a few bytes that
belong to the data of the previous ones, and a single call may emit up to
5 bytes more than expected. Let's just clarify this to avoid future
surprises.
Unexpectedly, an unsigned char is promoted to a *signed* int when
shifting, so shifting a value >= 128 by 24 places can overflow it, which
is undefined behaviour depending on build options. It happens to produce
the expected result with the usual compilers, but ubsan on an i386 build
reports it for about half of input bytes:
src/slz.c:482:107: runtime error: left shift of 220 by 24 places cannot
be represented in type 'int'
Let's just properly cast the uchars to u32 before shifting (this does
not change the produced code at all).
IMPORT: slz: fix the adler32 accumulators signedness on 32-bit
slz_adler32_block() unfortunately uses a signed long as the crc accumulator
instead of an unsigned one, meaning that for CRC values where the 32th bit
is set on 32-bit machines, the right shift will drag sign bits and corrupt
it. This only affects zlib streams on 32-bit systems (rfc1950) and has
been there for a very long time, showing that the zlib format is really
not much used in target environments.
The fix is trivial, just change the accumulators to unsigned long.
IMPORT: slz: do not append a block to an already finished stream
slz_rfc1951_flush() terminates the pending block then emits an empty
stored block to byte-align the output. But encode() called with <more>
cleared may leave the stream in SLZ_ST_LAST, that is, inside a block whose
BFINAL bit has already been sent. Flushing there terminated that block,
which completes the deflate stream, and then emitted one more block with
BFINAL set again. Those 5 bytes sit past the end of the stream, so the
gzip or zlib trailer that finish() appends right after is no longer where
the peer expects it: it reads the empty block as the checksum.
The data itself decodes correctly, only the check fails, which makes it
particularly difficult to diagnose. Raw deflate is not affected as it has
no trailer to shift, and a stream that ends in EOB state is not affected
either since its queue is empty and flush() returns early. Fuzzing random
sequences of encode/flush/finish showed ~2% corrupt streams for gzip and
zlib.
When the terminated block was the final one there is nothing left to align
to, and nothing may be appended, so let's just flush the pending bits and
return. This also makes the flush 4 bytes shorter in that case, and the
BFINAL bit of the empty block is now always zero, which is what the state
guarantees at that point.
IMPORT: slz: bound the bits wasted by the 9-bit literals
Commit libslz/002e838 ("bug: always make sure to limit fixed output to
less than worst case literals") made sure that switching from EOB to FIXED
to emit a reference is only done when the reference pays for the way back,
based on the fact that once in FIXED state a reference is always smaller
than the bytes it replaces.
This was unfortunately not enough: the literals interleaved with those
references can still fail. Indeed, octets 144 to 255 cost 9 bits instead
of the 8 they would cost in a stored block. The <bit9> counter measures
exactly that, but it was reset after every reference, so a stream
alternating just under 52 such literals with a cheap reference never
reached the threshold that sends them as a stored block, and kept
inflating for as long as the pattern lasted. 51 random literals >= 144
followed by 4 bytes copied from 8 bytes earlier (i.e. almost the exact
same pattern as previously tested) produce 1073161 bytes out of 1048576
(+2.34%) where the API promises at most 1048663, i.e. 24 kB more than a
caller sizing its output buffer from that promise would expect for a
single call.
Bit9 isn't sufficient to track the debt cross references, so let's add a
second <debt> counter to the stream's unused space. It accumulates the
bits actually wasted by the literals emitted in huffman mode, and each
reference records what it saved over the same bytes sent as literals,
bounded to zero. Above SLZ_MAX_DEBT (200 bits, i.e. 25 bytes) the encoder
stops trusting bit9: pending literals are stored and references have to
compensate for the full round trip, which lets the literal runs merge into
a full 65535-byte block and stop the growth.
The crafted stream now produces 1048677 bytes (+14 bytes over the
documented maximum instead of +24509). Other inputs such as text or
silesia corpus do not show any change since it's quite hard to fall
into this case.
Note that the threshold is deliberately much larger than the 52 bits of
a switch to amortize oscillations without needlessly sending literals.
IMPORT: slz: use the exact switch cost for the last literals of a block
The decision to send the pending literals as a stored block rather than in
fixed huffman mode is taken when the 9-bit literals wasted more than the
52 bits it costs to leave the fixed huffman encoding and to come back to
it. But for the last literals of a block, nothing comes after the stored
block, so there is no need to pay for the block type of a next block nor
for the EOB, while the huffman variant still has to send an EOB. The
switch is thus 10 bits cheaper, and 10 more when the stream is still in
EOB state, since then the block type is needed in both cases and no EOB
has to be terminated.
Using 52 there made the encoder prefer huffman for data that was cheaper
to store, and the output could exceed the documented maximum. The smallest
case found by fuzzing is a 47-byte input entirely made of bytes >= 144
which produced 55 bytes (3 bits of block type + 47*9 bits + 7 bits of EOB)
where the stored block only needs 52, for a documented maximum of 54.
With these correct costs, we no longer see outputs exceed the documented
maximum, wether it's with small inputs (tested with ~3 million random
small inputs as small as 47 bytes), or usual files found in tests/ and
bash, gcc, libc, and silesia. No performance change was observed either.
Note that a stream can still exceed the documented maximum by a few bytes
(17 bytes were observed on a 390000-byte crafted input) because each
reference emitted between two stored blocks forces them out and adds a
5-byte block header that the accounting attributes to the reference. This
is for a future fix.
IMPORT: slz: fix the documented worst case size of flush() and finish()
The documented output buffer requirements of the flush() and finish()
functions date back to the 32-bit queue, where at most 7 bits could be
pending. Since the 64-bit queue was introduced (used on x86_64 and armv8),
up to 31 bits may be pending, and the accounting also forgot the EOB that
may have to be emitted before the empty block. As a result a caller
strictly sizing its output buffer from the documentation could be short
by one to two bytes and see the encoder write past the end of its buffer.
Let's update the document worst cases for these functions depending on
what they still have to emit: 31 pending bits + 7 for EOB + 3 for
BFINAL/BTYPE + 7 for EOB or 32 for LEN+NLEN, rounded up to the next
byte (easily forgotten):
Note that all values are at least as large as the previously claimed ones
and that 32-bit systems never consume more than what was claimed, so the
new documented values are valid both for 32 and 64 bits.
Even though this patch only touches comments, it's marked as a bug so
that it is backported where it matters and users have a chance to spot
the new values.
IMPORT: slz: do not read past the end of the input around the match loop
slz_rfc1951_encode() pre-loads the first 3 bytes of the input into <word>
before entering the main loop on architectures which do not define
UNALIGNED_FASTER (e.g. i386, or big endian ones). This was done
unconditionally, thus inputs shorter than 3 bytes (including empty ones)
caused up to 3 bytes to be read past the end of the input buffer, which
may segfault if the buffer ends on the last page of a mapping. This is
easily reproduced on i386 by placing a zero-length input right before an
unmapped page.
The pre-loaded word is only ever used if the main loop is entered, which
requires at least 4 remaining bytes, so let's simply condition the load
on this.
The exact same case exists at the end of the loop where we can go beyond
end-3 and try to read 3 or 4 bytes before getting back to the beggining
of the loop, so we're using the same condition here, which helps the
compiler perform the test only once and use unconditional branches from
there.
The code is unchanged on x86_64 and armv8 (out of ifdef) and no
measurable change is observed on other archs.
This version provides the uslz (u stands for uncompress) API that allows
to decompress zlib/gzip compatible streams (including the ones generated
by slz) using 32K sliding buffer. It is meant for users that care about
memory footprint, but if performance is the only priority, using zlib is
perfectly fine for this.
Credits to Andrew Church <achurch@achurch.org> for providing
https://achurch.org/tinflate.c aka tiny inflate library as public domain.
libslz decompression implementation was greatly inspired from tinflate
library by Andrew Church.
zdec utility was updated to leverage this new API in order to act as a
fully operationnal decompression tool instead of simply dumping basic
debug infos like it used to do.
Shortlog:
Aurelien DARRAGON (5):
REORG: move generic internal function and helpers inside slz-prv.h file
MAJOR: reorganize libslz to split compression API code from common API code
MAJOR: slz: implement uncompress API (uslz)
REORG: rename zdec utility to zdecode
MINOR: add zdec decompression utility
Juan Pablo Mora [Thu, 6 Aug 2026 15:07:56 +0000 (17:07 +0200)]
REGTESTS: ssl: add ssl_c_policies test
Add a VTC covering both modes of ssl_c_policies(): the full list of
Certificate Policies OIDs, and the "-m found" single-OID check (both
a present and an absent OID).
reg-tests/ssl/certs/client1.pem (used by the other ssl_c_* sample
tests) is an X509v1 certificate and carries no extensions at all, so
it cannot be reused here. A dedicated, disposable CA and client
certificate (ca_policies.crt / client_policies.pem) were generated
for this test only, with a Certificate Policies extension containing
0.4.0.1862.1.4 and a filler OID. No personal or production data is
involved, only generic placeholder subject fields consistent with
the other certs in this directory.
Verified locally with vtest (VTest2, "last" tag), alongside
ssl_client_samples.vtc and ssl_client_auth.vtc to confirm no
regressions. checkpatch.pl clean (only the generic, inapplicable
"does MAINTAINERS need updating?" notice, since there is no SSL
maintainer entry).
Juan Pablo Mora [Thu, 6 Aug 2026 15:07:55 +0000 (17:07 +0200)]
MINOR: ssl: add ssl_c_policies sample fetch
Until now there was no way in HAProxy to inspect the "Certificate
Policies" X509v3 extension of a client certificate presented during
mTLS client auth. This is needed to take routing/access decisions
based on the policy under which the certificate was issued, e.g. to
tell apart eIDAS qualified certificates whose private key is held in
a QSCD (policy OID 0.4.0.1862.1.4, id-etsi-qcp-legal-qscd) from other
client certificates.
This adds ssl_c_policies([<oid>]), following the same extraction
pattern already used by ssl_c_san (X509_get_ext_d2i() +
comma-separated list built in a trash chunk):
- with no argument, it returns the full comma-separated list of
policy OIDs (numeric dotted form) found in the certificate ;
- with an <oid> argument, it only returns a sample when this
specific OID is present among the certificate's policies, which
allows using the "found" match method to take a decision, eg:
acl qualified_qscd ssl_c_policies(0.4.0.1862.1.4) -m found
http-request deny unless qualified_qscd
doc/configuration.txt is updated accordingly.
This is a pure addition, it does not touch any existing code path.
Built with -Wall -Wextra -Werror (no warnings) and validated against
doc/coding-style.txt's checkpatch.pl invocation: clean except for one
expected hit on the missing space in "ARG1(0,STR)", which matches the
pre-existing convention used by the other 22 entries of the same
sample_fetch_keywords table in this file.
The req.ssl_sni fetch (as well as all other req.ssl_* fetches, which
share the same underlying parsing) only analyzes the first ClientHello
message found in the request buffer.
This is not obvious to users, and can lead to incorrect assumptions
when the value is used for routing or access control decisions:
- if the client sends a second ClientHello within the same TCP
stream, for instance following a TLS 1.3 HelloRetryRequest, or
during a TLS renegotiation, that second ClientHello (and its SNI)
is silently ignored.
- when Encrypted Client Hello (ECH) is used, only the "Outer"
ClientHello is visible on the wire, so the SNI returned is a decoy
SNI and not the actual host the client intends to reach. The
"Inner" ClientHello, which carries the real SNI, is encrypted and
cannot be analyzed by this fetch.
Document these limitations on req.ssl_sni, and add a short pointer to
that documentation on the other req.ssl_* fetches (req.ssl_alpn,
req.ssl_cipherlist, req.ssl_ec_ext, req.ssl_hello_type,
req.ssl_keyshare_groups, req.ssl_sigalgs, req.ssl_st_ext,
req.ssl_supported_groups, req.ssl_ver).
BUG/MEDIUM: hlua_fcn: ensure systematic bref cleanup for patref list iterator
Similar bug as aeff2a3b2a ("BUG/MEDIUM: hlua_fcn: ensure systematic watcher
cleanup for server list iterator") but this time it affects patref list
iterator.
If the patref list iteration is interrupted (ie: break away from the loop
or lua error), we still need to unlink the bref we set earlier because
the hlua_patref_iterator_context is a temporary object so we cannot
let a reference once the object is dead. Obviously this can corrupt
the pattern reference element bref "users" list and lead to invalid
reads as well.
Reported-by: Claude (ANT-2026-DARC9AY8)
It should be backported up to 3.2 where hlua patref API was implemented.
BUG/MEDIUM: lua: resume Channel:send() from the unsent part of the string
hlua_channel_send_yield() keeps in <l> the number of bytes already pushed
into the channel across yields, and correctly limits the amount it tries
to push next to the remaining "sz - l" bytes. But it always passed the
beginning of the string to _hlua_channel_insert(), so after a partial
write the same prefix was sent again instead of the remainder.
For a Lua script calling Channel:send() with more data than the channel
can currently hold, the peer therefore receives the beginning of the
buffer twice and never sees its tail. Any protocol framing carried over
that stream is silently desynchronized, and the script has no way to
notice since the returned count is correct.
Let's advance the source pointer by the number of bytes already sent, as
the applet send paths do.
This was introduced in 2.5 by commit a1ac5fb28 ("MEDIUM: filters/lua: Be
prepared to filter TCP payloads"). It must be backported to all stable
versions.
BUG/MINOR: quic: drop multiple Retry on same connection
Ensures that only a single Retry packet is handled by a QUIC haproxy
client per connection. This is mandated by RFC 9000. The first received
token should still be sufficient to validate the connection.
This change is applied directly in quic_rx_pkt_parse(). In case of
multiple Retry, packets are silently ignored, whether token is identical
or not.
This fix is particularly important to prevent a memory leak on several
elements, first <retry_token> member of quic_conn. This also concerns
elements from the TLS stack as initial encryption level would be
reinitialized needlessly.
Reported-by: Claude (ANT-2026-CJ4Z875H)
This must be backported up to 3.3.
Force CRYPTO Rx buffer to be unaligned when running under stress. Most
of the time, this will cause a connection error with
CRYPTO_BUFFER_EXCEEDED as CRYPTO wrapping is not yet implemented.
BUG/MEDIUM: quic: prevent out-of-bound read on wrapping CRYPTO content
Received CRYPTO frames are buffered in a ncbmbuf to handle out-of-order
reception. When new content is available at the current offset, TLS
stack is notified so that it can read it. This is performed either via
ha_quic_ossl_crypto_recv_rcd() (for OpenSSL 3.5+) or
qc_ssl_provide_all_quic_data().
Depending on the receiving order, CRYPTO content may wrap over time.
This is currently not supported by haproxy as stated in a comment in
qc_ssl_provide_all_quic_data(), however there is no explicit code
protection to avoid it. Thus, a TLS stack may read past the CRYPTO
content as ncbmb_data() will report the size of data with wrapping
included.
The objective of this patch is to prevent any out-of-bound read attempt
by closing the connection on error. A check is added after buffering a
new CRYPTO frame in qc_handle_crypto_frm() : if content is wrapping, an
error CRYPTO_BUFFER_EXCEEDED is reported, the buffer is released and the
connection is closed. This happens before ha_quic_ossl_crypto_recv_rcd()
or qc_ssl_provide_all_quic_data() so this is a sufficient fix.
Note that the first idea was to directly patch
ha_quic_ossl_crypto_recv_rcd() / qc_ssl_provide_all_quic_data(). However
it is not easy as return value of these functions is ignored by their
callers.
Currently, this bug is unlikely as it's not possible to obtain a
condition for CRYPTO content to wrap with the available combination of
QUIC clients and their SSL library. It was reproduced only after manual
modification on ncbmb_init() to setup head buffer pointer near its end.
However, it's not guaranteed to not occur even without code patching so
the current fix is still necessary.
In the future, it may be necessary to complete this patch so that CRYPTO
wrapping can be realigned and decoded. A COUNT_IF() has been added to
help detect when this is the case.
Reported-by: Claude (ANT-2026-Y2QP9HED)
This should be backported up to 2.6.
Willy Tarreau [Thu, 6 Aug 2026 08:03:28 +0000 (10:03 +0200)]
BUG/MINOR: mux-h2: strip the userinfo when deriving :authority for a server
The H2 mux properly drops userinfo from authority on input but doesn't
drop it on output if present on input (e.g. coming from H1), which will
cause a bad request when reaching a compliant H2 server such as itself.
Let's make sure it is properly dropped there as well, as required by
RFC9113. This should be backported to all stable versions.
Willy Tarreau [Fri, 7 Aug 2026 13:06:53 +0000 (15:06 +0200)]
BUG/MINOR: debug: only dump the trace once in __BUG_ON_ONCE()
Amaury found that CHECK_IF() was dumping the stack trace on each call,
which was not the intent. The reason is that complain() performs the
check on the counter, but the decision to dump the stack later is based
on the choice between aborting and dumping. Let's break out of this when
the count is > 1. Under high concurrency it can cause a trace never to
be emitted but this is a detail. The whole thing needs to be redone
more cleanly anyway.
Willy Tarreau [Thu, 6 Aug 2026 07:31:15 +0000 (09:31 +0200)]
BUG/MINOR: lb-chash: bound the walk when the saved cursor changed tree
chash.last is not reset when the tree in use switches from the backup
servers back to the active ones. The next call then starts walking the
active tree while keeping a backup node as the stop condition, which it can
never reach, so it cycles over the active tree forever as soon as no server
can be picked: all of them saturated (maxconn reached with a queue, or
served >= the dynamic maxconn), or the only one left being the server to
avoid on a redispatch. In this case, if chash.last (assigned to <stop>)
isn't in the current tree, the loop will run forever.
Let's just count the number of times we wrap and stop at the second,
which indicates that the stop server is not in the tree.
Note that this is more theoretical than practical: this needs
"hash-type consistent" with requests not carrying the hash key so that
this fallback is used at all, backup servers with "option allbackups"
(without it lbprm.fbck is returned directly and the backup tree is never
walked), and all active servers going down then one coming back while
the remaining ones are saturated. In practice it has been there since
consistent hash was introduced in 1.4 by commit 6b2e11be1 ("[MEDIUM]
backend: implement consistent hashing variation") and was never
reported. It may be backported to all stable versions.
Willy Tarreau [Thu, 6 Aug 2026 07:27:02 +0000 (09:27 +0200)]
BUG/MEDIUM: session: don't release a reversed connection twice on error
A reversed rhttp connection already has its mux installed when it
reaches session_accept_fd(). If an error occurs after setup (e.g. a
"tcp-request connection reject" rule, or conn_xprt_start() failing),
the error path calls session_free(), which destroys the mux and thus
releases the connection. Control then reaches out_free_conn, which
calls conn_release() on it a second time, causing a crash.
Stop after session_free() when the connection has a mux, since it's
already handled. The listener still needs explicit release.
This dates back to reverse-http's introduction in 3.0 (12c40c25a,
"MEDIUM: rhttp: create session for active preconnect"). Must be
backported to 3.0.
Willy Tarreau [Thu, 6 Aug 2026 07:27:02 +0000 (09:27 +0200)]
BUG/MINOR: connection: reserve the whole CRC32C TLV before saving its pointer
make_proxy_line_v2() saves a pointer to the CRC32C TLV value so the
checksum can be appended once the whole header is known. It only checked
for 3 bytes (the TLV header) instead of the 7 needed for the full TLV, so
with 3 to 6 bytes left, make_tlv() emitted nothing and returned 0, yet
tlv_crc32c_p was still set, and the final write_u32() wrote 4 bytes up to
3 bytes past the end of the header buffer.
Not only this theoretically only affects servers using "send-proxy-v2"
with "proxy-v2-options crc32c" combined with "set-proxy-v2-tlv-fmt" TLVs,
but in addition in practice the proxy protocol is designed to be way
shorter than a regular buffer, and the only practical way to reach that
is to purposely write a config to demonstrate this, so it cannot happen.
This has been there since crc32c support was added in 1.9 by commit 4399c75f6 ("MINOR: proxy-v2-options: add crc32c"). It should be backported
to all stable versions.
Dragan Dosen [Fri, 7 Aug 2026 11:08:45 +0000 (11:08 +0000)]
BUG/MEDIUM: fd: release the port range entry in host byte order
The port recovered with getsockname() in _fd_delete_orphan() was given
back to the port range without being converted to host byte order. The
range then fills up with byte-swapped values which are handed out on the
next pass. Both TCP and QUIC are affected.
It is now released using get_host_port(), which takes care of the
conversion for both address families.
The issue was introduced with commit 02b7685013 ("MEDIUM: fd: Remove
fdinfo").
Commit c81d794822cf8fa3c2e163cbf4cba80ad0dd9a47 made it so we could have
one pipe pool per thread group, however it did not limit how many pipes
we'd allocate per thread group, so one thread group could end up having
all the pipes. Prevent that by allowing a maximum of maxpipes / nbtgroup
per thread group.
Document that when multi-queue is used with tune.fd.tables set to
per-thread-group, then connections will only be load-balanced across
threads from the same thread group.
MINOR: backend: Do not always allow takeover across thread groups
Instead of silently ignoring tune.idle-pool.shared. full
if tune.fd.tables per-thread-group is set, have an error at startup if
both are set, as they are incompatible.
BUG/MINOR: acme: restrict the permissions of the generated account key
When no ACME account key exists yet, haproxy generates one and writes it
through a plain BIO_new_file(), so the file is created with the process'
umask applied. With the common 022 umask the unencrypted private key ends
up on disk as 0644, readable by every local user.
Whoever reads that key can authenticate to the CA as this haproxy ACME
account, and from there deactivate it or manipulate the orders and
revocations for the domains it has validated.
Let's restrict the file to 0600 right after creating it and before writing
anything into it.
This was introduced in 3.2 by commit 856b6042d ("MEDIUM: acme: generate
the account file when not found"). It must be backported to 3.2.
BUG/MINOR: ssl: reject server certificate names containing a NUL byte
ssl_sock_srv_verifycbk() decodes each SAN dNSName and each CN of the
server certificate with ASN1_STRING_to_UTF8(), which returns the decoded
length, then passes the result to the hostname matcher as a plain
NUL-terminated C string and throws the length away. A name encoded as
"victim.com\0.attacker.com" is therefore compared as "victim.com" and
matches.
This defeats the point of "verify required" together with "verifyhost" or
SNI on a server line: an attacker holding a certificate with such a name,
and able to intercept the connection to the backend, passes the name check
and can read and alter all the proxied traffic. This is the CVE-2009-2408
class of bug. It requires a CA to issue such a certificate, which modern
CAs refuse to do, so the practical risk is low, but the check is cheap.
Let's compare the decoded length with strlen() and ignore any name that
does not match.
This has been there since "verifyhost" was introduced in 1.5 by commit be55431f9 ("MINOR: ssl: Add statement 'verifyhost' to "server"
statements"). It may be backported to all stable versions.
BUG/MEDIUM: acme: don't delete a NULL token from the map
When an ACME task ends, acme_del_acme_ctx_map() walks the list of
authorizations and removes each challenge token from the configured map.
But an authorization is created with only its URL set, and its token is
only filled in once the CA's answer for that authorization has been
successfully parsed. The "dns-persist-01" challenge type never sets one at
all. Every termination path of the task goes through that cleanup, so a
NULL token was passed to pat_ref_delete(), which compares it against the
keys of the reference and dereferences it.
For users this means that an ACME configuration using a "map" crashes the
worker whenever the certificate renewal fails early, for instance when the
CA is unreachable and the retries are exhausted, or when it answers
something unexpected. The very component that is supposed to keep the
service running then takes it down.
Let's simply ignore authorizations without a token, they have nothing
registered in the map anyway.
This was introduced in 3.2 by commit 5555926fd ("MEDIUM: acme: use a map
to store tokens and thumbprints"). It must be backported to 3.2.
BUG/MEDIUM: ssl: require a full-length AEAD tag when decrypting with AES-GCM
aes_process() passes the caller-provided tag length straight to
EVP_CTRL_AEAD_SET_TAG, and OpenSSL accepts and verifies GCM tags as short
as one byte. In the "aes_gcm_dec" converter the tag argument is documented
as coming from a variable, which in practice is populated from request
data, and in the JWE AES-GCM key-wrap path it comes directly from the
token. In both cases the party submitting the ciphertext also chooses how
many bytes of the authentication tag get checked.
An attacker submitting a one-byte tag therefore only needs about 256
attempts, instead of 2^128, to have arbitrary ciphertext accepted as
authentic. For a configuration relying on these converters to validate a
signed or encrypted token, this is a full authentication bypass.
The encrypt side always emits a 16-byte tag, so let's simply require
exactly that on the decrypt side.
This has been there since these converters were introduced, the shared
helper coming from commit f0e64de75 ("MINOR: ssl: Factorize AES GCM data
processing") in 3.4. It must be backported to all stable versions
providing "aes_gcm_dec".
BUG/MINOR: jwt: don't take an extra reference on the certificate public key
X509_get_pubkey() already returns an owned reference, so the extra
EVP_PKEY_up_ref() on the cert-store path leaked one reference per
verification, since only one EVP_PKEY_free() is done afterwards. Over
time this means key objects accumulate and are never released, even
across certificate reloads.
Drop the extra reference. The "jwt_cert_tree" path below still needs
its up_ref, since it only holds a borrowed pointer.
Introduced in 3.3 by 522bca98e ("MAJOR: jwt: Allow certificate instead
of public key in jwt_verify converter"). Must be backported to 3.3.
BUG/MEDIUM: jwe: validate the secret length against the algorithm of the token
The "alg" field of the JOSE header of a JWE token selects the key-wrapping
algorithm, hence the AES key size, while the key itself is the secret
configured by the operator for the "jwt_decrypt_secret" converter. That
secret is stored in an exact-size heap allocation, and neither
decrypt_cek_aeskw() nor aes_process() (used by the AES-GCM key wrap
variant) checked that it was large enough for the selected cipher before
handing its address to OpenSSL.
So a client sending a token that declares A256KW or A256GCMKW while the
configured secret is only 16 bytes long makes OpenSSL read 16 bytes past
the end of that allocation and use them as key material. This is a
remotely triggered heap over-read which may crash the worker, and whose
bytes influence the decryption result.
Let's check the secret length against the cipher's key length in both
paths before initializing the cipher.
Both were introduced in 3.4, by commits f0e64de75 ("MINOR: ssl: Factorize
AES GCM data processing") and 416b87d5d ("MINOR: jwe: Add new
jwt_decrypt_secret converter"). This must be backported to 3.4.
Reported-by: Claude (ANT-2026-TS9WFQ5T) Reported-by: Claude (ANT-2026-9T34RNDD)
BUG/MEDIUM: log: always reserve room for trailing 0 when using CBOR encoding helpers
Logging helpers leveraged by sess_build_logline_orig() can be split in
two different groups. Although they all look similar in their construction
as they take pretty much the same parameters and return the address where
following bytes can be appended, some will always try to append the
terminating NULL byte and return the address of the terminating NULL
byte, while others (which are not specifically text oriented) will simply
use all available space (they don't reserve space for the terminating NULL
byte) and return the address of the byte following the last byte written.
But since they don't try to write the \0 themselves, they will in practise
output one extra byte compared to other helpers. If no precaution is taken
and they are used as drop-in replacement to text oriented ones, this can
cause invalid writes later in the code because sess_build_logline_orig()
will always append the terminating NULL byte (even if it is already set),
thus it is mandatory that the output pointer never reaches the stopmark.
Fortunately, most pitfalls were already avoided in log generation path,
but recent commit c614fd3b ("MINOR: log: add +cbor encoding option")
made use of several encoding helpers which were not text oriented as
text oriented ones. Let's fix that by always securing 1 byte for the
terminating NULL byte when calling them (even if it not used by the
endpoint X format, sess_build_logline_orig() will append it not matter
what, so we have to live with that).
Reported-by: Claude (ANT-2026-QQ17FDX1)
It should be backported up to 3.0.
MINOR: log/tools: fix ambiguous comments for some log encoding helpers
Some log encoding helpers are not text oriented, and they will use all
available space since they will not try to write a terminating NULL
byte at the end of the produced output themselves. But since they
work similarly to text oriented ones, they return the address of
the byte immediately following the payload (where we expect
following bytes to be written). For text oriented ones this corresponds
in fact to the terminating NULL byte which was accounted in the
available space, while for non text-oriented ones, which doesn't reserve
space for the terminating NULL byte this corresponds to 1 byte past the
payload. If available space is strictly the size of the produced output,
then it means the returned address will be 1 byte PAST the stop limit so
no extra bytes could be written anymore. When using this helpers, callers
have to be very careful to reserve bytes (ie: terminating NULL byte) if
they need to.
It may be backported up to 3.0. Before that such ambiguities didn't exist
as logging features were strictly text-oriented and log encoders were not
available.
BUG/MINOR: mux-fcgi: sanitize the STDERR records before logging them
fcgi_strm_handle_stderr() emits one log line per STDERR record, appending
its own newline, but passes the record payload to app_log() with a bare
"%s", and neither app_log() nor __send_log() escape anything. FastCGI
applications routinely echo parts of the request in their warnings, so a
client whose input is reflected there can insert CR/LF and turn one record
into several log lines, or insert ESC sequences which the operator's
terminal interprets when reading the log. Verified with an application
writing "bad input 'x\r\nFAKE-INJECTED-LINE: ...\033[31m...'": the syslog
datagram carries all of it verbatim.
Escaping data emitted to logs is normally a configuration matter, but this
path bypasses the log-format machinery entirely, and the intent here is
clearly one record per line. Let's replace the control characters with a
dot before logging, which is a single pass over a record that is only ever
produced when the application writes to its stderr. Only controls are
replaced, so that the UTF-8 messages commonly found in such warnings are
left intact.
This has been there since the FCGI mux was introduced in 2.1 by commit 99eff65f4 ("MEDIUM: mux-fcgi: Add the FCGI multiplexer"). It may be
backported to all stable versions.
BUG/MEDIUM: http-ana: check the cookie rewrite result before moving the offsets
In the "rewrite" and "prefix" cookie modes,
http_manage_server_side_cookies() calls http_replace_header_value() and
ignores its return value. When the expansion doesn't fit it returns 0 and
leaves ctx.value untouched, yet the code still computes <delta> from
srv->cklen, advances next and hdr_end by it, and in "prefix" mode writes
"val_beg[srv->cklen] = COOKIE_DELIM" over data that was never moved. The
delimiter thus lands in whatever follows the cookie and the response is
forwarded with that byte corrupted, while the cookie is left unprefixed,
silently losing persistence.
The maxrewrite reserve normally covers the expansion, so this needs the
inserted string to be larger than it. Reproduced both with a 1100-byte
server cookie name and the default tune.maxrewrite, and with a 61-byte one
and "tune.maxrewrite 16": the Set-Cookie comes back untouched and a '~'
appears in the middle of the next header's value.
Let's check the return value like every other rewrite site does: on
failure nothing is touched, the failed_rewrites counters are incremented
and the parsing of that response's cookies stops there.
This has been there since the HTX cookie handling was added in 1.9 by
commit fcda7c685 ("MINOR: proto_htx: Add functions to manage cookies on
HTX messages"). It may be backported to all stable versions.
BUG/MEDIUM: sock: bound the recvmsg() length when receiving old sockets
sock_get_old_sockets() sizes tmpbuf from the number of FDs announced by the
old process, but passes a fixed iov_len of MAX_SEND_FD entries to every
recvmsg() and loops as long as fewer FDs than announced were received,
without ever comparing curoff to the size of the allocation. A peer
announcing a single FD (4118 bytes allocated) and then streaming plain data
with no SCM_RIGHTS makes the kernel write up to 252*4118 bytes per recvmsg()
past the end of the buffer, and the loop never ends. Reproduced with a fake
old process: glibc aborts on "free(): invalid next size" after ~320 kB. Only
the peer of the -x transfer socket can do this, so it is not reachable from
the network, but it happens before privileges are dropped.
Let's clamp each recvmsg() to the room really left in the allocation and
abort the transfer when the peer sends more. Legitimate transfers are
unaffected, they use at most 1+255+1+255+4 bytes per FD.
This has been there since commit f73629d23 ("MINOR: global: Add an option to
get the old listening sockets.") in 1.8, which already sized tmpbuf on fd_nb
and the iovec on MAX_SEND_FD. It may be backported to all stable versions.