Heikki Linnakangas <hlinnakan...@vmware.com> writes:

> On 11/24/2014 06:05 PM, Alex Shulgin wrote:
>> The first patch is not on topic, I just spotted this missing check.
>
>> *** a/src/interfaces/libpq/fe-connect.c
>> --- b/src/interfaces/libpq/fe-connect.c
>> *************** conninfo_array_parse(const char *const *
>> *** 4402,4407 ****
>> --- 4402,4415 ----
>>                                                              if 
>> (options[k].val)
>>                                                                      
>> free(options[k].val);
>>                                                              options[k].val 
>> = strdup(str_option->val);
>> +                                                            if 
>> (!options[k].val)
>> +                                                            {
>> +                                                                    
>> printfPQExpBuffer(errorMessage,
>> +                                                                            
>>                           libpq_gettext("out of memory\n"));
>> +                                                                    
>> PQconninfoFree(options);
>> +                                                                    
>> PQconninfoFree(dbname_options);
>> +                                                                    return 
>> NULL;
>> +                                                            }
>>                                                              break;
>>                                                      }
>>                                              }
>
> Oh. There are actually many more places in connection option parsing
> that don't check the return value of strdup(). The one in fillPGConn
> even has an XXX comment saying it probably should check it. You can
> get quite strange behavior if one of them fails. If for example the
> strdup() on dbname fails, you might end up connecting to different
> database than intended. And if the "conn->sslmode =
> strdup(DefaultSSLMode);" call in connectOptions2 fails, you'll get a
> segfault later because at least connectDBstart assumes that sslmode is
> not NULL.
>
> I think we need to fix all of those, and backpatch. Per attached.

Yikes!  Looks sane to me.

I've checked that patches #2 and #3 can be applied on top of this, w/o
rebasing.

--
Alex


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to