Le 06/11/2026 à 2:26 AM, Martin Mirchev a écrit :
Hi Christopher,
Thanks for merging the tcpcheck fix. Here is a v2 of the two other
patches, which should now cover all the cases, against current master
(64d597032b).
1/2 BUG/MINOR: mjson: fix overreads on truncated JSON paths
Besides the trailing backslash of v1, an array index without its
closing ']' also makes mjson_get_cb() read past the end of the
path: "$[0", "$[" and "$[]" through the fixed 3-character skip, and
"$[1" or "$.a[1" through the unbounded search for ']'. Both
searches now stop at the end of the string, as upstream mjson does
since 2021. The fix is in the walker, so it covers json_query,
jwt_header_query, jwt_payload_query and the other mjson callers. I
can also reject such paths in sample_check_json_query() if you
prefer.
2/2 BUG/MINOR: stick-table: reject invalid gpc/gpt array indexes
v1 only rejected negative indexes in the sc-* actions and on the
CLI. v2 checks the documented range (0 to 99) wherever an index is
parsed: the sc-inc-gpc, sc-set-gpt and sc-add-gpc actions, the CLI
data.gpc[]/data.gpt[] names, and the 16 fetches and converters
taking an index (sc_get_gpc, src_get_gpt, table_gpc...), through a
shared argument check. Indexes above UINT_MAX were truncated, e.g.
sc-inc-gpc(4294967296,0) incremented gpc[0]. The CLI table filters
now also reject an index beyond the table's array, like "set table".
Tested with a release build (TARGET=linux-glibc USE_OPENSSL=1
USE_PCRE2=1 USE_ZLIB=1, no warnings) and an ASan+UBSan build. On master,
json_query and jwt_header_query report a heap-buffer-overflow READ in
plen1() or mjson_get_cb() for the truncated paths. With the patch there
is no report and well-formed paths give the same results. Each of the
19 index keywords is rejected with -1, 100 and 4294967296 and accepted
with 0 and 99, and a valid configuration gives the same counters at
runtime. "haproxy -c" gives the same result before and after on the
examples/ configs and on the 416 configs embedded in the reg-tests,
including sc_gpc_gpt, json_query and the jwt ones. I still could not run
the reg-tests themselves.
The patches are attached (git format-patch -v2).
Thanks, I merged the stick-table one. For the other one, because I'm unable to
judge, I asked to my LLM and it produces a version a bit stricker than yours.
Following paths return an empty result while with your patch a matching is
performed:
http-request set-var(txn.b1) req.fhdr(xjson),json_query($.a[1)
http-request set-var(txn.b2) req.fhdr(xjson),json_query($.a[)
http-request set-var(txn.b3) req.fhdr(xjson),json_query($.a[x])
http-request set-var(txn.b4) req.fhdr(xjson),json_query('$.a[ 1]')
http-request set-var(txn.b5) req.fhdr(xjson),json_query('$.a[1 ]')
http-request set-var(txn.b6) req.fhdr(xjson),json_query($.a[+1])
http-request set-var(txn.b7) req.fhdr(xjson),json_query($.a[1x])
http-request set-var(txn.r1) req.fhdr(xarr),json_query($[1)
http-request set-var(txn.r2) req.fhdr(xarr),json_query($[0)
With this kind of request:
curl -s -H 'xjson: {"a":[10,20,30]}' -H 'xarr: [10,20,30]'...
nothing is matched with my version. 10 or 20 is matched with yours. So I'm
tempted to merge my version. But it is really a blind shot. It could be good to
double-check.
Regards,
--
Christopher Faulet
From a0bf87c6f21cf6864a2cfa0fd0bada05eb8a6545 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Fri, 30 Oct 2026 13:32:48 +0800
Subject: [PATCH] BUG/MINOR: mjson: fix out-of-bounds reads on malformed JSON
paths
The path passed to mjson_find() is parsed without always staying within
its bounds, so a malformed path can read past its end. This is a
heap-buffer-overflow whenever the path is stored in an exactly sized
allocation, which is the case of the json_query converter's argument
since it is duplicated by my_strndup() at configuration parsing time.
Two code paths are involved.
plen1() and plen2() measure a 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 then reads past the end of
the string. With:
http-request set-var(txn.out) req.fhdr(json),json_query($.a\\)
(the config parser turns "\\" into a single backslash, so the path is
"$.a\" followed by the terminating zero). Every request carrying a JSON
object in the "json" header then makes plen1() read past the end of the
5-byte allocation. ASan reports a heap-buffer-overflow READ of size 1 in
plen1(), reached from sample_conv_json_query(). Paths without a
trailing backslash are not affected.
Then mjson_get_cb() locates the array index either by skipping a fixed 3
bytes from the path position when the index is 0, or by scanning
forward for a ']' with no bound otherwise. A truncated index breaks
both: for "$.a[" and "$.a[0", the fixed skip lands past the trailing
zero, and for "$.a[1", the scan walks past the end of the string until
it happens to meet a ']' further in the heap. Nothing validates the
path at configuration parsing time, so such a configuration passes
"haproxy -c" and only fails when the converter is evaluated on a
matching request.
These paths come from the configuration only: the JWE and ACME code
uses hardcoded paths or snprintf() with a bounded index, and the other
callers pass constant strings. So the bug is only reachable when the
configuration itself is malformed, which limits its severity.
Let's only skip the character following a backslash when it is not the
end of the string, in both plen1() and plen2(), and move the array index
parsing to a new skip_idx() helper. It only accepts a decimal number
immediately followed by a ']', and never scans past the end of the
string so that a truncated index is reported as not found. This is
slightly stricter than the previous code, which accepted "+1", "1 " and
"1x" as index 1 because mystrtod() stops at the first character that is
not part of the number, while "$.a[00]" is now recognized as index 0
instead of being skipped as a 3-byte suffix.
The plen1() and plen2() 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.
Co-authored-by: Martin Mirchev <[email protected]>
---
src/mjson.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/src/mjson.c b/src/mjson.c
index 603947d72..9a94ed553 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,11 +184,25 @@ 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;
}
+// Moves <*pos> past the array index it points to, i.e. from the opening '['
+// to the character following the closing ']'. Returns 0 if the index is not
+// a decimal number properly terminated by a ']', and leaves <*pos> untouched.
+// The scan never goes past the end of the string so that a truncated index
+// such as "$.a[" or "$.a[0" cannot cause an overread.
+static int skip_idx(const char *path, int *pos) {
+ int i = *pos + 1;
+ if (path[i] < '0' || path[i] > '9') return 0;
+ while (path[i] >= '0' && path[i] <= '9') i++;
+ if (path[i] != ']') return 0;
+ *pos = i + 1;
+ return 1;
+}
+
static int kcmp(const char *a, const char *b, int n) {
int i = 0, j = 0, r = 0;
for (i = 0, j = 0; j < n; i++, j++) {
@@ -213,21 +227,16 @@ static int mjson_get_cb(int tok, const char *s, int off, int len, void *ud) {
if (data->d1 == data->d2 && data->path[data->pos] == '[') {
data->i1 = 0;
data->i2 = (int) mystrtod(&data->path[data->pos + 1], strlen(&data->path[data->pos + 1]), NULL);
- if (data->i1 == data->i2) {
+ if (data->i1 == data->i2 && skip_idx(data->path, &data->pos))
data->d2++;
- data->pos += 3;
- }
}
if (!data->path[data->pos] && data->d1 == data->d2) data->obj = off;
data->d1++;
} else if (tok == ',') {
if (data->d1 == data->d2 + 1) {
data->i1++;
- if (data->i1 == data->i2) {
- while (data->path[data->pos] != ']') data->pos++;
- data->pos++;
+ if (data->i1 == data->i2 && skip_idx(data->path, &data->pos))
data->d2++;
- }
}
} else if (tok == MJSON_TOK_KEY && data->d1 == data->d2 + 1 &&
data->path[data->pos] == '.' && s[off] == '"' &&
--
2.56.0
