Re: jsonb iterator not fully initialized

From: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
To: Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: jsonb iterator not fully initialized
Date: 2018-05-26 14:47:16
Message-ID: 15062a71-5c88-bbeb-3b5d-c56dd57aa56c@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 05/26/2018 03:09 AM, Piotr Stefaniak wrote:
> On 2018-05-26 02:02, Peter Eisentraut wrote:
>> I got this error message via -fsanitized=undefined:
>>
>> jsonfuncs.c:5169:12: runtime error: load of value 127, which is not a
>> valid value for type '_Bool'
>>
>> The query was
>>
>> select ts_headline('{}'::jsonb, tsquery('aaa & bbb'));
>>
>> This calls the C function ts_headline_jsonb_byid_opt(), which calls
>> transform_jsonb_string_values(), which calls
>>
>> it = JsonbIteratorInit(&jsonb->root);
>> is_scalar = it->isScalar;
>>
>> but it->isScalar is not always initialized by JsonbIteratorInit(). (So
>> the 127 is quite likely clobbered memory.)
>>
>> It can be fixed this way:
>>
>> --- a/src/backend/utils/adt/jsonb_util.c
>> +++ b/src/backend/utils/adt/jsonb_util.c
>> @@ -901,7 +901,7 @@ iteratorFromContainer(JsonbContainer *container,
>> JsonbIterator *parent)
>> {
>> JsonbIterator *it;
>>
>> - it = palloc(sizeof(JsonbIterator));
>> + it = palloc0(sizeof(JsonbIterator));
>> it->container = container;
>> it->parent = parent;
>> it->nElems = JsonContainerSize(container);
>>
>> It's probably not a problem in practice, since the isScalar business is
>> apparently only used in the array case, but it's dubious to leave things
>> uninitialized like this nonetheless.
>>
> I've seen it earlier but couldn't decide what my proposed fix should
> look like. One of the options I considered was:
>
> --- a/src/backend/utils/adt/jsonfuncs.c
> +++ b/src/backend/utils/adt/jsonfuncs.c
> @@ -5010,10 +5010,8 @@ transform_jsonb_string_values(Jsonb *jsonb, void
> *action_state,
> JsonbIteratorToken type;
> JsonbParseState *st = NULL;
> text *out;
> - bool is_scalar = false;
>
> it = JsonbIteratorInit(&jsonb->root);
> - is_scalar = it->isScalar;
>
> while ((type = JsonbIteratorNext(&it, &v, false)) != WJB_DONE)
> {
> @@ -5033,7 +5031,7 @@ transform_jsonb_string_values(Jsonb *jsonb, void
> *action_state,
> }
>
> if (res->type == jbvArray)
> - res->val.array.rawScalar = is_scalar;
> + res->val.array.rawScalar =
> JsonContainerIsScalar(&jsonb->root);
>
> return JsonbValueToJsonb(res);
> }

palloc0 seems cleaner.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2018-05-26 15:06:31 Re: [PATCH] Clear up perlcritic 'missing return' warning
Previous Message Michael Paquier 2018-05-26 14:42:38 Re: SCRAM with channel binding downgrade attack