Hi,
Here are two config checks that turn a runtime crash into a parsing error,
and a small cleanup, against current master (3e758361c). The first two
come with a reg-test. The patches are attached (git format-patch).
This series is independent of the "[PATCH 0/2] two small config parsing
fixes (thread ranges, lone '-' in strl2irc)" series I sent earlier today.
The two series touch different files and apply on top of each other in
either order.
1/3 BUG/MINOR: cfgparse-listen: reject negative capture lengths
"capture request header X len -1", "capture response header X len -1"
and "capture cookie X len -1" are accepted ("declare capture" and
"http-request capture" already reject len <= 0), and the first
matching message corrupts memory. For headers, the pool is sized
len+1, and isttrim(v, -1) does not trim, so the whole header value is
copied into it. ASan reports a heap-buffer-overflow WRITE from
http_capture_headers(), and a release build usually dies soon after
with a pool tag mismatch, a malloc() abort or a SIGSEGV. For the
cookie, memcpy() is called with (size_t)-1. "len 4294967295"
truncates to -1 and does the same. The patch rejects negative lengths
and lengths that do not fit in an int. I kept "len 0" accepted, since
it is harmless and some configs may use it. If you would rather match
"http-request capture" and reject 0 too, I can send a v2.
2/3 BUG/MINOR: stick-table: reject negative stick counter IDs in sc-*
actions
sc-inc-gpc(0,-2), sc-inc-gpc0(-1), sc-add-gpc(0,-1), sc-set-gpt(0,-1)
and sc-set-gpt0(-2) are accepted because the ID is only checked
against tune.stick-counters. At runtime the actions read
s->stkctr[-2] or s->stkctr[-1]. ASan reports a heap-buffer-overflow
READ in stkctr_entry(), and the release build segfaults or aborts for
some of them. GitHub #3061 (21d5f43aa) fixed the upper bound for
sc-add-gpc, but not the lower one. The sc_* fetches and track-sc are
not affected, because they use unsigned numbers.
3/3 CLEANUP: resolvers: don't close the nameserver socket twice on deinit
resolvers_destroy() calls fd_delete(), which already closes the FD,
and then close() on the same FD. With strace, each nameserver socket
shows a second close() returning EBADF on soft-stop.
Testing (Docker, Ubuntu 24.04, gcc 13, TARGET=linux-glibc USE_OPENSSL=1
USE_ZLIB=1 USE_PCRE2=1, plus ASan builds of master and of the patched
tree): the crashes above were reproduced on master by sending one request
through the proxy, and with the patches the configs are rejected. The new
reg-tests fail on master and pass with the patches. The full "make
reg-tests" run gives 317 passed and 1 failed with the patches, and 314
passed and 2 failed on master. The failure seen on both is
ssl/wrong_ctx_storage.vtc, which is environmental (a command is missing
in the image). The other failure on master was stickiness/srvkey-addr.vtc,
which passed 5 times out of 5 when rerun alone. Patches 1 and 2 should be
backported to all stable branches. 3.4, 3.2 and 2.8 have the same code;
in those branches the capture parser is still in cfg_parse_listen().
This issue was found by an LLM-Enhanced Static Analysis Engine and verified
manually.
Thanks,
Martin Mirchev
From 305ec7389a420180eda4828a650a5b554d95dcb6 Mon Sep 17 00:00:00 2001
From: Martin Mirchev <[email protected]>
Date: Tue, 27 Oct 2026 14:37:07 +0800
Subject: [PATCH 1/3] BUG/MINOR: cfgparse-listen: reject negative capture
lengths
"capture request header", "capture response header" and "capture cookie"
read their length with atol() and never check it, unlike "declare capture"
and "http-request capture" which reject lengths <= 0. A negative length,
or one that does not fit in an int such as 4294967295, is thus accepted,
and the first matching message crashes the process:
- for headers, the capture pool is created with size len+1 (0 for -1),
then http_capture_headers() calls isttrim(v, h->len), which converts
-1 to SIZE_MAX and does not trim anything, so the whole header value
is copied into the tiny pool object. ASan reports a heap-buffer-overflow
WRITE from http_capture_headers(), and a release build usually dies
soon after with a pool tag mismatch, a malloc() corruption abort or a
SIGSEGV;
- for the cookie, http_manage_client_side_cookies() clamps log_len to
capture_len (-1) and calls memcpy() with (size_t)-1 (ASan:
negative-size-param).
Let's reject negative lengths and lengths that do not fit in an int at
parsing time. A length of zero is still accepted since it is harmless
(it captures an empty string) and some configurations may rely on it. A
reg-test is added.
This should be backported to all stable branches. The parser was moved
to proxy_parse_capture() in 3.5-dev, older branches have the same atol()
calls in cfg_parse_listen().
---
reg-tests/startup/capture_negative_len.vtc | 55 ++++++++++++++++++++++
src/cfgparse-listen.c | 17 ++++++-
2 files changed, 70 insertions(+), 2 deletions(-)
create mode 100644 reg-tests/startup/capture_negative_len.vtc
diff --git a/reg-tests/startup/capture_negative_len.vtc b/reg-tests/startup/capture_negative_len.vtc
new file mode 100644
index 000000000..3cd4af83d
--- /dev/null
+++ b/reg-tests/startup/capture_negative_len.vtc
@@ -0,0 +1,55 @@
+varnishtest "capture request/response header and capture cookie reject a negative length"
+
+# A negative length used to be accepted by "capture request header",
+# "capture response header" and "capture cookie", then made haproxy write
+# past the capture buffer (headers) or memcpy() (size_t)-1 bytes (cookie)
+# on the first matching message.
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf-BAD {invalid length '-1'} {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ capture request header X-Foo len -1
+}
+
+haproxy h2 -conf-BAD {invalid length '-1'} {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ capture response header X-Foo len -1
+}
+
+haproxy h3 -conf-BAD {invalid length '-1'} {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ capture cookie SID len -1
+}
+
+haproxy h4 -conf-BAD {invalid length '4294967295'} {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ capture request header X-Foo len 4294967295
+}
+
+haproxy h5 -conf-OK {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ capture request header X-Foo len 0
+ capture response header X-Foo len 15
+ capture cookie SID len 32
+}
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index a7207a764..0d3284b0b 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -1189,6 +1189,7 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
char **err)
{
struct cap_hdr *hdr;
+ long len;
warnifnotcap(curpx, PR_CAP_FE, file, line, args[0], NULL);
@@ -1208,6 +1209,12 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
return -1;
}
+ len = atol(args[4]);
+ if (len < 0 || len > INT_MAX) {
+ memprintf(err, "'%s %s': invalid length '%s'.", args[0], args[1], args[4]);
+ return -1;
+ }
+
name = strdup(args[2]);
if (!name)
goto alloc_error;
@@ -1215,7 +1222,7 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
free(curpx->capture_name);
curpx->capture_name = name;
curpx->capture_namelen = strlen(name);
- curpx->capture_len = atol(args[4]);
+ curpx->capture_len = len;
curpx->to_log |= LW_COOKIE;
return 0;
}
@@ -1231,6 +1238,12 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
return -1;
}
+ len = atol(args[5]);
+ if (len < 0 || len >= INT_MAX) {
+ memprintf(err, "'%s %s %s': invalid length '%s'.", args[0], args[1], args[2], args[5]);
+ return -1;
+ }
+
hdr = calloc(1, sizeof(*hdr));
if (!hdr)
goto alloc_error;
@@ -1240,7 +1253,7 @@ static int proxy_parse_capture(char **args, int section_type, struct proxy *curp
goto alloc_err_free_hdr;
hdr->namelen = strlen(args[3]);
- hdr->len = atol(args[5]);
+ hdr->len = len;
hdr->pool = create_pool("caphdr", hdr->len + 1, MEM_F_SHARED);
if (!hdr->pool)
goto alloc_err_free_name;
--
2.50.1
From fedff1d8923b3add63b2a04913bca3d58a284af2 Mon Sep 17 00:00:00 2001
From: Martin Mirchev <[email protected]>
Date: Tue, 27 Oct 2026 14:37:07 +0800
Subject: [PATCH 2/3] BUG/MINOR: stick-table: reject negative stick counter IDs
in sc-* actions
The parsers of sc-inc-gpc, sc-inc-gpc0/1, sc-add-gpc, sc-set-gpt and
sc-set-gpt0 read the stick counter ID with strtol() into a signed int and
only check it against tune.stick-counters. A negative ID such as in
"sc-inc-gpc(0,-2)" or "sc-set-gpt0(-1)" is thus accepted, and at runtime
the actions read &s->stkctr[-2] or &sess->stkctr[-1], before the start of
the stick counters array. ASan reports a heap-buffer-overflow READ in
stkctr_entry() called from the action, and a release build may crash or
use whatever lies there as a stick-table entry pointer to update it.
The sc_* sample fetches are not affected since they convert the ID to an
unsigned int before the check, and track-sc* parses an unsigned number.
Let's also reject negative IDs. A reg-test is added.
This should be backported to all stable branches. sc-add-gpc() appeared
in 2.8 and the others are older.
---
reg-tests/stick-table/sc_negative_id.vtc | 65 ++++++++++++++++++++++++
src/stick_table.c | 10 ++--
2 files changed, 70 insertions(+), 5 deletions(-)
create mode 100644 reg-tests/stick-table/sc_negative_id.vtc
diff --git a/reg-tests/stick-table/sc_negative_id.vtc b/reg-tests/stick-table/sc_negative_id.vtc
new file mode 100644
index 000000000..6ac530201
--- /dev/null
+++ b/reg-tests/stick-table/sc_negative_id.vtc
@@ -0,0 +1,65 @@
+varnishtest "sc-inc-gpc/sc-add-gpc/sc-set-gpt reject a negative stick counter ID"
+
+# The stick counter ID of these actions was only checked against the upper
+# bound, so a negative one was accepted and used as an index before the
+# stick counters array at runtime.
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf-BAD {invalid stick table track ID} {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ http-request sc-inc-gpc(0,-2)
+}
+
+haproxy h2 -conf-BAD {invalid stick table track ID} {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ http-request sc-inc-gpc0(-1)
+}
+
+haproxy h3 -conf-BAD {invalid stick table track ID} {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ http-request sc-add-gpc(0,-1) 1
+}
+
+haproxy h4 -conf-BAD {invalid stick table track ID} {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ http-request sc-set-gpt(0,-1) 1
+}
+
+haproxy h5 -conf-BAD {invalid stick table track ID} {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ http-request sc-set-gpt0(-2) 1
+}
+
+haproxy h6 -conf-OK {
+ defaults
+ mode http
+ timeout client 30s
+ frontend fe
+ bind "fd@${fe}"
+ http-request sc-inc-gpc(0,0)
+ http-request sc-inc-gpc0(1)
+ http-request sc-add-gpc(0,2) 1
+ http-request sc-set-gpt(0,0) 1
+ http-request sc-set-gpt0(1) 1
+}
diff --git a/src/stick_table.c b/src/stick_table.c
index 9c447bb99..dbb59f7e0 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -2914,7 +2914,7 @@ static enum act_parse_ret parse_inc_gpc(const char **args, int *arg, struct prox
return ACT_RET_PRS_ERR;
}
- if (rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
+ if (rule->arg.gpc.sc < 0 || rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
memprintf(err, "invalid stick table track ID '%s'. The max allowed ID is %d (tune.stick-counters)",
args[*arg-1], global.tune.nb_stk_ctr-1);
return ACT_RET_PRS_ERR;
@@ -2942,7 +2942,7 @@ static enum act_parse_ret parse_inc_gpc(const char **args, int *arg, struct prox
return ACT_RET_PRS_ERR;
}
- if (rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
+ if (rule->arg.gpc.sc < 0 || rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
memprintf(err, "invalid stick table track ID. The max allowed ID is %d (tune.stick-counters)",
global.tune.nb_stk_ctr-1);
return ACT_RET_PRS_ERR;
@@ -3144,7 +3144,7 @@ static enum act_parse_ret parse_set_gpt(const char **args, int *arg, struct prox
return ACT_RET_PRS_ERR;
}
- if (rule->arg.gpt.sc >= global.tune.nb_stk_ctr) {
+ if (rule->arg.gpt.sc < 0 || rule->arg.gpt.sc >= global.tune.nb_stk_ctr) {
memprintf(err, "invalid stick table track ID '%s'. The max allowed ID is %d",
args[*arg-1], global.tune.nb_stk_ctr-1);
return ACT_RET_PRS_ERR;
@@ -3170,7 +3170,7 @@ static enum act_parse_ret parse_set_gpt(const char **args, int *arg, struct prox
return ACT_RET_PRS_ERR;
}
- if (rule->arg.gpt.sc >= global.tune.nb_stk_ctr) {
+ if (rule->arg.gpt.sc < 0 || rule->arg.gpt.sc >= global.tune.nb_stk_ctr) {
memprintf(err, "invalid stick table track ID '%s'. The max allowed ID is %d",
args[*arg-1], global.tune.nb_stk_ctr-1);
return ACT_RET_PRS_ERR;
@@ -3337,7 +3337,7 @@ static enum act_parse_ret parse_add_gpc(const char **args, int *arg, struct prox
return ACT_RET_PRS_ERR;
}
- if (rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
+ if (rule->arg.gpc.sc < 0 || rule->arg.gpc.sc >= global.tune.nb_stk_ctr) {
memprintf(err, "invalid stick table track ID '%s'. The max allowed ID is %d (tune.stick-counters)",
args[*arg-1], global.tune.nb_stk_ctr - 1);
return ACT_RET_PRS_ERR;
--
2.50.1
From 9a0bbf5b39073aa81df7a63309519604509bd0d1 Mon Sep 17 00:00:00 2001
From: Martin Mirchev <[email protected]>
Date: Tue, 27 Oct 2026 14:29:44 +0800
Subject: [PATCH 3/3] CLEANUP: resolvers: don't close the nameserver socket
twice on deinit
resolvers_destroy() calls fd_delete() on the UDP socket of each nameserver,
which already closes it, then calls close() on the same FD again. This
second close() fails with EBADF (visible with strace on soft-stop), and
could close an unrelated FD if the number were reused in between. Let's
just drop the redundant close().
This came with resolvers_destroy() in 2.7 (commit e606c84fe "MINOR:
resolvers: resolvers_destroy() deinit and free a resolver"). There is no
need to backport it, though it does not hurt.
---
src/resolvers.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/resolvers.c b/src/resolvers.c
index 66006b663..8b78ee341 100644
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -2765,7 +2765,6 @@ static void resolvers_destroy(struct resolvers *resolvers)
(resolvers->t &&
ha_thread_info[resolvers->t->tid].tgid == tgid))) {
fd_delete(ns->dgram->conn.t.sock.fd);
- close(ns->dgram->conn.t.sock.fd);
}
dns_ring_free(ns->dgram->ring_req);
free(ns->dgram);
--
2.50.1
