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(at)vondra(dot)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(at)gmail(dot)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