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 00:14:01 -0400

Daniel Stenberg wrote:
> On Thu, 22 Aug 2013, Daniel Stenberg wrote:
>
>> I guess that information, if each line was individually allocated or
>> not, needs to be stored somewhere so that _ldap_free_urldesc can do
>> the right thing.
>
> This thread seems to have died, but I'm attaching my patch here with a
> slightly different approach to fixing the problem.
>

Sorry for letting the thread sit so long! I've been on the road lately
and hadn't gotten to put together an alternate patch.

> I'll appreciate some feedback and preferably some testing of this since
> I don't have a dev system that actually uses this code!
>

I think this approach is sane, but there are a couple of issues with the
patch.

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().

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.

I've confirmed both of the above in a debugger.

I think the way to finish the fix using this approach is probably to use
the attribute count gathered after the loop on line 619 (following the
call to split_str()) to initialize ludp->lud_attrs_dup, and bracket the
freeing of those elements in if(ludp->lud_attrs_dup inside
_ldap_free_urldesc(). If that sounds right to you, I'll try to put
something together a later this week.

My system for putting a patch together is a little cumbersome at the
moment because I don't have git on the Windows system where I can test
this fix.

(As an aside, do you know of a good git package for Windows that's not
based on msys? I'm afraid to install msys-git on there because that
system already has a load-bearing installation of msys and I really
don't want to troubleshoot any conflicts that might occur if I try to
make the two coexist.)

Thanks,

Geoff
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-09-05