Re: [HACKERS] Fix for bug in ldapServiceLookup in libpq
Tom Lane wrote: > You missed one "return" where the string needed to be freed. I've > applied this patch with that fix and a couple of cosmetic changes. > Thanks for the report and patch! Thanks for the work and the keen eye! Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for bug in ldapServiceLookup in libpq
"Albe Laurenz" writes: > I have attached a new version of the patch that should address all known > problems. You missed one "return" where the string needed to be freed. I've applied this patch with that fix and a couple of cosmetic changes. Thanks for the report and patch! regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for bug in ldapServiceLookup in libpq
Tom Lane wrote: >> I have found a small but annoying bug in libpq where >> connection parameters are resolved via LDAP. > Hmm ... that's a bug all right, but why have the null-termination > inside the loop at all? Seems like it should look like > > for (p = result, i = 0; values[i] != NULL; ++i) > { > strncpy(p, values[i]->bv_val, values[i]->bv_len); > p += values[i]->bv_len; > *(p++) = '\n'; > } > *p = '\0'; Yes, that's better. > ... btw, shouldn't this function free the "result" string when it's done > with it? AFAICS that string is not returned to the caller, it's just > being leaked. Oops, yes it should. > (I'll refrain from asking why it's creating the string in the first > place rather than parsing ldap_get_values_len's output as-is ...) So that I can close the LDAP connection as soon as feasible, but of course that's not absolutely necessary. I have attached a new version of the patch that should address all known problems. Yours, Laurenz Albe ldapServiceLookup.patch Description: ldapServiceLookup.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for bug in ldapServiceLookup in libpq
... btw, shouldn't this function free the "result" string when it's done with it? AFAICS that string is not returned to the caller, it's just being leaked. (I'll refrain from asking why it's creating the string in the first place rather than parsing ldap_get_values_len's output as-is ...) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix for bug in ldapServiceLookup in libpq
"Albe Laurenz" writes: > I have found a small but annoying bug in libpq where > connection parameters are resolved via LDAP. > There is a write past the end of a malloc'ed string which causes > memory corruption. The code and the bug are originally by me :^( Hmm ... that's a bug all right, but why have the null-termination inside the loop at all? Seems like it should look like for (p = result, i = 0; values[i] != NULL; ++i) { strncpy(p, values[i]->bv_val, values[i]->bv_len); p += values[i]->bv_len; *(p++) = '\n'; } *p = '\0'; > This should be backpatched to 8.2 where the code was introduced. Yes, will do. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Fix for bug in ldapServiceLookup in libpq
I have found a small but annoying bug in libpq where connection parameters are resolved via LDAP. There is a write past the end of a malloc'ed string which causes memory corruption. The code and the bug are originally by me :^( The attached patch fixes the problem in HEAD. This should be backpatched to 8.2 where the code was introduced. Yours, Laurenz Albe ldapServiceLookup.patch Description: ldapServiceLookup.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers