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