Re: BUG #18598: AddressSanitizer detects use after free inside json_unique_hash_match()

On Wed, Sep 4, 2024 at 3:21 PM Tomas Vondra <tomas@vondra.me> wrote:
>
> On 9/1/24 21:00, PG Bug reporting form wrote:
> > The following bug has been logged on the website:
> >
> > Bug reference:      18598
> > Logged by:          Alexander Lakhin
> > Email address:      exclusion@gmail.com
> > PostgreSQL version: 17beta3
> > Operating system:   Ubuntu 22.04
> > Description:
> >
> > The following query:
> > SELECT JSON_OBJECTAGG(i: (i)::text FORMAT JSON WITH UNIQUE)
> >  FROM generate_series(1, 100000) i;
> >
> > triggers an asan-detected error:
> > ==973230==ERROR: AddressSanitizer: heap-use-after-free on address
> > 0x7fde473f4428 at pc 0x558af80f20a6 bp 0x7ffe6b8e2df0 sp 0x7ffe6b8e2598
> > READ of size 7 at 0x7fde473f4428 thread T0
> >     #0 0x558af80f20a5 in __interceptor_strncmp.part.0
> > (.../usr/local/pgsql/bin/postgres+0x32d40a5)
> >     #1 0x558af9ed5276 in json_unique_hash_match
> > ...
> >
> > Reproduced starting from 7081ac46a.
> >
>
> FWIW I can reproduce this using valgrind, with the same stacks reported.
>
> This feels very much like a classical memory context bug - pointing to
> memory in a short-lived memory context. I see datum_to_json_internal()
> allocates the result in ExprContext, and that's bound to be reset pretty
> often. But I'm not too familiar with the JSON aggregate stuff enough to
> pinpoint what it does wrong.
>
> regards
>
> --
> Tomas Vondra
>
>

ISTM that the JsonUniqueHashEntry.key point to an address later got
invalidated by enlargeStringInfo, we can resolve this by explicitly
pstrdup the key in the same MemoryContext of JsonAggState, like:

@@ -1009,6 +1009,7 @@ json_object_agg_transfn_worker(FunctionCallInfo fcinfo,
        Datum           arg;
        bool            skip;
        int                     key_offset;
+       const char *key;

        if (!AggCheckCallContext(fcinfo, &aggcontext))
        {
@@ -1111,7 +1112,9 @@ json_object_agg_transfn_worker(FunctionCallInfo fcinfo,

        if (unique_keys)
        {
-               const char *key = &out->data[key_offset];
+               oldcontext = MemoryContextSwitchTo(aggcontext);
+               key = pstrdup(&out->data[key_offset]);
+               MemoryContextSwitchTo(oldcontext);


--
Regards
Junwang Zhao