cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: PATCH: prevent a double free() with a malformed LDAP URL

From: Geoff Beier <geoff_at_redhoundsoftware.com>
Date: Thu, 05 Sep 2013 18:03:50 -0400

Daniel Stenberg wrote:
>> First, in the case of a well-formed URL, ludp->lud_attrs_dup is not
>> initialized prior calling
>> ludp->lud_attrs_dup[i] = curl_easy_unescape(data, ludp->lud_attrs[i],
>> 0, NULL);
>> inside unescape_elements().
>
> The struct is allocated with calloc() in _ldap_url_parse(), is it not?
>
Yes. When the struct is allocated using calloc(), that allocates space
for a pointer and sets the pointer to 0.

Nothing allocates memory for the array, however, so the pointer
lud_attrs_dup is still 0x00 when the code above uses the [] operator.
That causes an attempt to dereference 0. I confirmed that this is
happening and causes a crash.

>> Second, in the case of a malformed URL, this loop:
>> + for(i = 0; ludp->lud_attrs_dup[i]; i++)
>> + free(ludp->lud_attrs_dup[i]);
>> runs in _ldap_free_urldesc() regardless of whether ludp->lud_attrs_dup
>> has been initialized.
>
> ... thus they should be NULL if not inited and free() works on NULL.
> Even if we in general avoid free on NULL in libcurl to better catch
> mistakes so I'd be fine with adding a check.
>
It's not freeing NULL that causes a crash. It's dereferencing NULL using
the [] operator that causes a crash.

I think we just need to allocate the array after we count the elements
in ludp->lud_attrs, check to make sure the array's been allocated before
using the [] operator to free its elements, and make sure inside
_ldap_free_urldesc() to free ludp->lud_attrs_dup if it's been allocated.

I've attached a modified version of your patch to show what I mean.
That's been tested on win32, and confirmed expected behavior for both
malformed and well-formed ldap urls.

(I just noticed that my editor on Windows messed up the indentation, and
I flubbed the subject line when using git format-patch, but I don't have
time to address that right at this moment. If this otherwise looks good,
I can fix that and resend if you want.)

Thanks,

Geoff

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html

  • text/plain attachment: stored
Received on 2013-09-06