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

On Wed, Sep 4, 2024 at 7:54 PM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>
> On 9/4/24 11:55, Junwang Zhao wrote:
> > ...
> >
> > 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:
>
> Yes, this fixes the issue (at least per valgrind).
>
> > @@ -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);
> >
>
> I think you don't need the new key declaration (there's already a local
> one), and you can simply do just
>
> const char *key = MemoryContextStrdup(aggcontext,
> &out->data[key_offset]);
>
> I wonder if the other json_unique_check_key() call might have a similar
> issue. I've not succeeded in constructing a broken query, but perhaps
> you could give it a try too?

I found two other places called json_unique_check_key.

One is *json_build_object_worker*, and the usage is the same as
*json_object_agg_transfn_worker*, I fix that the same way, PSA

The following sql should trigger the problem, I haven't tried asan
but traced the *repalloc* in gdb, I will try this later when I set up my
asan building.

SELECT JSON_OBJECT(1: 1, '2': NULL, '3': 1, repeat('x', 1000): 1, 2:
repeat('a', 100) WITH UNIQUE);

The other place is json_unique_object_field_start, it is set
as a callback of JsonSemAction, and in the parse state machine,
the fname used by the callback has been correctly handled.
see [1] & [2].

[1]: https://github.com/postgres/postgres/blob/master/src/common/jsonapi.c#L1069-L1087
[2]: https://github.com/postgres/postgres/blob/master/src/common/jsonapi.c#L785-L823

>
>
> Thanks!
>
> --
> Tomas Vondra

--
Regards
Junwang Zhao