Hi Christopher!

Thank you for merging the stick-table fix. I checked your mjson version
against master (64d597032b) and against my v2, with an ASan+UBSan build and
the json_query reproducer, using the headers from your mail plus a nested
one ({"a":[{"b":1},[5,6],{"c":[7,8]}]}).

It fixes the issue. On master, the paths ending with a backslash ("$.a\",
"$.b\") and the truncated indexes ("$.a[", "$.a[0", "$.a[1", "$.a[]", "$[",
"$[0", "$[1", "$[]", "$.a[2].c[1", "$.a[1][0") all give a
heap-buffer-overflow READ. With your patch none of them reports anything,
and all of them return an empty result.

Well-formed paths give the same results as on master and with my v2: "$.a",
"$.a[0]" to "$.a[3]", "$[0]" to "$[2]", "$.a[00]", "$.a[01]", "$.a[1][0]",
"$.a[1][1]", "$.a[0].b" and "$.a[2].c[1]". I did not find a problem in the
code: skip_idx() never reads past the trailing zero, and when it fails the
element is simply not matched, since i1 only grows after that. The build
has no warnings. I still could not run the reg-tests themselves.

The only behaviour change is the one you describe: "$.a[x]" (index 0 on
master) and "$.a[1 ]", "$.a[+1]", "$.a[1x]" (index 1 on master) now match
nothing. As these only come from a malformed configuration, I think the
stricter version is better, so I am fine with you merging yours instead of
mine.

Thank you!
Martin

On Fri, Nov 06, 2026 04:57 PM, Christopher Faulet <[email protected]>
wrote:

> 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

Reply via email to