389731 - PerLDAP crashes when a bad URL is passed

Status: RESOLVED FIXED

(Directory :: PerLDAP, defect)

Description

[Noriko Hosoi]

Sample input to cause the problem: "ldap://:<port>/<suffix>"
where <host> is missing.

Here's the stacktrace from the core
$ gdb `which perl` core.###
(gdb) bt
#0  0x00552b36 in Perl_newSVpv ()
  from /usr/lib/perl5/5.8.5/i386-linux-thread-multi/CORE/libperl.so
#1  0x002ba66f in XS_Mozilla__LDAP__API_ldap_url_parse (my_perl=0x9804008,  cv=0x9812d5c) at API.c:3027
#2  0x0054ca22 in Perl_pp_entersub ()
  from /usr/lib/perl5/5.8.5/i386-linux-thread-multi/CORE/libperl.so
#3  0x0052fedd in Perl_runops_debug ()
  from /usr/lib/perl5/5.8.5/i386-linux-thread-multi/CORE/libperl.so
#4  0x004e1c91 in perl_run ()
  from /usr/lib/perl5/5.8.5/i386-linux-thread-multi/CORE/libperl.so
#5  0x080493b2 in main ()
(gdb) up
#1  0x002ba66f in XS_Mozilla__LDAP__API_ldap_url_parse (my_perl=0x9804008,  cv=0x9812d5c) at API.c:3027
3027                  SV* host = newSVpv(realcomp->lud_host,0);
(gdb) p realcomp
$1 = (LDAPURLDesc *) 0x9df68d8
(gdb) p realcomp->lud_host
$2 = 0x0
(gdb) p url
$3 = 0x9d31920 "ldap://:<port>/<suffix>"

It looks newSVpv crashes if NULL is passed to the first argument.  XS_Mozilla__LDAP__API_ldap_url_parse calls newSVpv with these 2 args w/o checking the value.
         SV* host = newSVpv(realcomp->lud_host,0);
         SV* filter = newSVpv(realcomp->lud_filter,0);

Should we do something like this?
Index: API.xs
===================================================================
RCS file: /cvsroot/mozilla/directory/perldap/API.xs,v
retrieving revision 1.18.2.11
diff -t -w -U 4 -r1.18.2.11 API.xs
--- API.xs      14 Jun 2007 09:21:14 -0000      1.18.2.11
+++ API.xs      26 Jul 2007 20:31:57 -0000
@@ -1683,9 +1683,9 @@
           HV*   FullHash = newHV();
           RETVAL = newRV((SV*)FullHash);

           ret = ldap_url_parse(url,&realcomp);
-           if (ret == 0)
+           if (ret == 0 && realcomp->lud_host && realcomp->lud_filter)
           {
              static char *host_key = "host";
              static char *port_key = "port";
              static char *dn_key = "dn";

Comment 1

[Rich Megginson]

Attachment: slightly different fix

ldap_url_parse allows the host to be empty.  So we have to handle this case the same way we handle the empty dn case - just set host to an empty string.  I don't think we need to handle the filter differently - the code in url.c ldap_url_parse() in the LDAP C library will set filter to "(objectclass=*)" if NULL.

Comment 2

[Rich Megginson]

Attachment: cvs commit log