Hi, Here are three small independent fixes against current master (e5e52e33c). All three are about values from the configuration or the CLI that were not checked. The patches are attached (git format-patch).
1/3 BUG/MINOR: tcpcheck: check the number of "hdr" in http-check send The headers of an "http-check send" rule go into a stack array sized by tune.http.maxhdr plus a terminator, without any bounds check. With "tune.http.maxhdr 5" and five "hdr" arguments, "haproxy -c" segfaults on a release build (ASan: dynamic-stack-buffer-overflow WRITE). The default of 101 cannot be reached because of the 64-word line limit. The rule is now rejected with "too many headers". Backport to all stable versions (since 2.2). 2/3 BUG/MINOR: mjson: fix overread on a path ending with a backslash plen1()/plen2() skip two characters on a backslash, so a json_query path ending with one, e.g. json_query($.a\\), makes them read past the trailing zero. ASan reports a heap-buffer-overflow READ in plen1() on the first request. The upstream mjson code has the same bug. Backport to all stable versions (since 2.4). 3/3 BUG/MINOR: stick-table: reject negative gpc/gpt array indexes strtoul() accepts "-1" and wraps it, so data.gpc[-1] on the CLI and sc-inc-gpc(-1,0)/sc-set-gpt(-1,0)/sc-add-gpc(-1,0) in the config are accepted and then silently do nothing or fail as "index out of range". There is no memory issue, since the index is checked before use. This is the index counterpart of 1f1b701fe3. Backport to all stable versions, the CLI part only to 3.2 and above. Tested on e5e52e33c with an ASan+UBSan build and a release build (USE_OPENSSL=1 USE_PCRE2=1 USE_ZLIB=1). On master, the reproducers for 1/3 and 2/3 trigger the ASan reports above (1/3 also segfaults on the release build), and the invalid indexes of 3/3 are accepted. With the patches, 1/3 and 3/3 give a config or CLI error, and 2/3 returns an empty result with no ASan report. Paths with escaped separators ($.my\\.key) and array indexes still work. "haproxy -c" gives the same result before and after on the examples/ configs and on the configs embedded in the checks, stick-table, stickiness, converter and sample_fetches reg-tests. I could not run the reg-tests themselves because I do not have vtest here. These were found with an LLM-Enhanced Static Analysis Engine and verified manually. Martin Mirchev
From 79afdca813f906a2d6d06105ab5b90e4e5cf6798 Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Fri, 30 Oct 2026 13:32:30 +0800 Subject: [PATCH 1/3] BUG/MINOR: tcpcheck: check the number of "hdr" in http-check send parse_tcpcheck_send_http() collects the "hdr <name> <value>" arguments of an "http-check send" rule into a stack array sized by tune.http.maxhdr, followed by an IST_NULL terminator, but never checks the number of headers against that size. When tune.http.maxhdr is lowered and a rule sets as many headers as the limit, the terminator is written past the end of the array, and each extra header writes one more entry past it. This happens while parsing the configuration. E.g. with: global tune.http.maxhdr 5 backend b option httpchk http-check send meth GET uri /health hdr X-H1 v1 hdr X-H2 v2 \ hdr X-H3 v3 hdr X-H4 v4 hdr X-H5 v5 server s1 127.0.0.1:1 check "haproxy -c" crashes with a segfault on a release build, and ASan reports a dynamic-stack-buffer-overflow WRITE in parse_tcpcheck_send_http(). With the default value (101) it cannot be reached because a config line is limited to 64 words. Let's reject the rule with an error when there are too many headers, keeping room for the terminator. This should be backported to all stable versions. The code dates back to 2.2 and commit e5870d872b ("MAJOR: checks: Implement HTTP check using tcp-check rules"). --- src/tcpcheck.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/tcpcheck.c b/src/tcpcheck.c index 1e245e0c8..7861218f8 100644 --- a/src/tcpcheck.c +++ b/src/tcpcheck.c @@ -3090,6 +3090,13 @@ struct tcpcheck_rule *parse_tcpcheck_send_http(char **args, int cur_arg, struct strcasecmp(args[cur_arg+1], "transfer-encoding") == 0) goto skip_hdr; + /* keep room for the terminating IST_NULL entry */ + if (i >= global.tune.max_http_hdr - 1) { + memprintf(errmsg, "too many headers (tune.http.maxhdr is %d)", + global.tune.max_http_hdr); + goto error; + } + hdrs[i].n = ist(args[cur_arg + 1]); hdrs[i].v = ist(args[cur_arg + 2]); i++; -- 2.50.1
From e9f65512bcec62fc7e88367bf77f4c3e85c4613b Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Fri, 30 Oct 2026 13:32:48 +0800 Subject: [PATCH 2/3] BUG/MINOR: mjson: fix overread on a path ending with a backslash plen1() and plen2() measure a JSON path segment and skip two characters when they see a backslash, so that an escaped '.' or '[' is not taken as a separator. When the path ends with a backslash, the second skipped character is the trailing zero, and the loop goes on reading past the end of the string. The path comes from the json_query converter's argument, which is stored in an exactly sized allocation. With: http-request set-var(txn.out) req.fhdr(json),json_query($.a\\) (the config parser turns "\\" into "\", so the path is "$.a\"), any request carrying a JSON object in the "json" header makes plen1() read past the end of the 5-byte path. ASan reports a heap-buffer-overflow READ of size 1 in plen1() called from sample_conv_json_query(). Paths without a trailing backslash are not affected. Let's only skip the character following a backslash when it is not the end of the string, in both functions. The bug is still present in the upstream mjson project. This should be backported to all stable versions. The code came with mjson in 2.4. --- src/mjson.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mjson.c b/src/mjson.c index 603947d72..97a6f34b0 100644 --- a/src/mjson.c +++ b/src/mjson.c @@ -176,7 +176,7 @@ struct msjon_get_data { static int plen1(const char *s) { int i = 0, n = 0; while (s[i] != '\0' && s[i] != '.' && s[i] != '[') - n++, i += s[i] == '\\' ? 2 : 1; + n++, i += (s[i] == '\\' && s[i + 1] != '\0') ? 2 : 1; // printf("PLEN: s: [%s], [%.*s] => %d\n", s, i, s, n); return n; } @@ -184,7 +184,7 @@ static int plen1(const char *s) { static int plen2(const char *s) { int i = 0, __attribute__((unused)) n = 0; while (s[i] != '\0' && s[i] != '.' && s[i] != '[') - n++, i += s[i] == '\\' ? 2 : 1; + n++, i += (s[i] == '\\' && s[i + 1] != '\0') ? 2 : 1; // printf("PLEN: s: [%s], [%.*s] => %d\n", s, i, s, n); return i; } -- 2.50.1
From 5cc67f103578520844306accf595b3394f2e9e88 Mon Sep 17 00:00:00 2001 From: Martin Mirchev <[email protected]> Date: Fri, 30 Oct 2026 13:33:07 +0800 Subject: [PATCH 3/3] BUG/MINOR: stick-table: reject negative gpc/gpt array indexes The array index of gpc, gpc_rate and gpt is parsed with strtoul(), which accepts a leading '-' and negates the result. Thus an index of -1 is silently turned into a huge unsigned value instead of being rejected: - on the CLI, "set table <t> key <k> data.gpc[-1] 7" fails with "index out of range in this data array", exactly like data.gpc[4294967295], instead of "Unknown data type" as for data.gpc[q]. "show table <t> data.gpc[-1] gt 0" is accepted as a filter and matches nothing. - in the configuration, "sc-inc-gpc(-1,0)", "sc-set-gpt(-1,0)" and "sc-add-gpc(-1,0)" pass "haproxy -c", and the action then does nothing at runtime since stktable_data_ptr_idx() finds the index out of range. There is no memory access issue since the index is checked against the array size before use, but the invalid value should be reported where it is parsed. Let's require the index to start with a digit, both in stktable_get_data_type_idx() and in the sc-inc-gpc, sc-set-gpt and sc-add-gpc action parsers. This is the index counterpart of commit 1f1b701fe3 ("BUG/MINOR: stick-table: reject negative stick counter IDs in sc-* actions"). This should be backported to all stable versions. The CLI part only applies to 3.2 and above where stktable_get_data_type_idx() was added, sc-add-gpc() appeared in 2.8, and the other actions in 2.5. --- src/stick_table.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/stick_table.c b/src/stick_table.c index dbb59f7e0..dbe1c80dd 100644 --- a/src/stick_table.c +++ b/src/stick_table.c @@ -1996,6 +1996,8 @@ int stktable_get_data_type_idx(char *name, unsigned int *idx) /* we've got a match */ name += stop + 1; + if (!isdigit((unsigned char)*name)) + return -1; // bad value (strtoul() would accept "-1") *idx = strtoul(name, &ret, 10); if (ret == name || *ret != ']') return -1; // bad value @@ -2902,7 +2904,7 @@ static enum act_parse_ret parse_inc_gpc(const char **args, int *arg, struct prox if (*cmd_name == '(') { cmd_name++; /* skip the '(' */ rule->arg.gpc.idx = strtoul(cmd_name, &error, 10); /* Convert stick table id. */ - if (*error != ',') { + if (!isdigit((unsigned char)*cmd_name) || *error != ',') { memprintf(err, "Missing gpc ID '%s'. Expects sc-inc-gpc(<GPC ID>,<Track ID>)", args[*arg-1]); return ACT_RET_PRS_ERR; } @@ -3132,7 +3134,7 @@ static enum act_parse_ret parse_set_gpt(const char **args, int *arg, struct prox if (*cmd_name == '(') { cmd_name++; /* skip the '(' */ rule->arg.gpt.idx = strtoul(cmd_name, &error, 10); /* Convert stick table id. */ - if (*error != ',') { + if (!isdigit((unsigned char)*cmd_name) || *error != ',') { memprintf(err, "Missing gpt ID '%s'. Expects sc-set-gpt(<GPT ID>,<Track ID>)", args[*arg-1]); return ACT_RET_PRS_ERR; } @@ -3325,7 +3327,7 @@ static enum act_parse_ret parse_add_gpc(const char **args, int *arg, struct prox } cmd_name++; /* skip the '(' */ rule->arg.gpc.idx = strtoul(cmd_name, &error, 10); /* Convert stick table id. */ - if (*error != ',') { + if (!isdigit((unsigned char)*cmd_name) || *error != ',') { memprintf(err, "Missing gpc ID. Expects %s(<GPC ID>,<Track ID>)", args[*arg-1]); return ACT_RET_PRS_ERR; } -- 2.50.1
