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

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?


Thanks!

-- 
Tomas Vondra