On Thu, 5 Sep 2013, Geoff Beier wrote:

Thanks for the feedback!

> 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?

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

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

And what problems do they cause?

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

Or just break the loop on the first NULL pointer?

I've attached an updated version of the patch that does that and also it keeps
the original struct layout before the new 'lud_attrs_dup' as it struck me
ldap_url_parse() probably needs that - if anyone ever actually builds this
code with HAVE_LDAP_URL_PARSE defined.

-- 
  / daniel.haxx.se

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