commit 3dc6fc42bfc61b86d5cb4632bdd3c2f47fb357af
author Geoff Beier <geoff@redhoundsoftware.com> Thu Sep 05 17:51:53 2013 -0400
committer Daniel Stenberg <daniel@haxx.se> Fri Sep 06 13:32:49 2013 +0200
tree 51d7a8f191be3ea72174b24d895602ae8a006799
parent d2fe616e7e44a106ac976aaeaa441ad7d8a6df11

LDAP: fix bad free() when URL parsing failed

When an error occurs parsing an LDAP URL, The ludp->lud_attrs[i] entries
could be freed even though they sometimes point to data within an
allocated area.

This change introduces a lud_attrs_dup[] array for the duplicated string
pointers, and it removes the unused lud_exts array.

Bug: http://curl.haxx.se/mail/lib-2013-08/0209.html

lib/ldap.c

diff --git a/lib/ldap.c b/lib/ldap.c
index 2352715..450b302 100644
--- a/lib/ldap.c
+++ b/lib/ldap.c
@@ -77,13 +77,14 @@
 /* Use our own implementation. */
 
 typedef struct {
-    char   *lud_host;
-    int     lud_port;
-    char   *lud_dn;
-    char  **lud_attrs;
-    int     lud_scope;
-    char   *lud_filter;
-    char  **lud_exts;
+  char   *lud_host;
+  int     lud_port;
+  char   *lud_dn;
+  char  **lud_attrs;
+  int     lud_scope;
+  char   *lud_filter;
+  char  **lud_exts;
+  char  **lud_attrs_dup; /* gets each entry malloc'ed */
 } CURL_LDAPURLDesc;
 
 #undef LDAPURLDesc
@@ -364,7 +365,7 @@
   }
 
   rc = ldap_search_s(server, ludp->lud_dn, ludp->lud_scope,
-                     ludp->lud_filter, ludp->lud_attrs, 0, &result);
+                     ludp->lud_filter, ludp->lud_attrs_dup, 0, &result);
 
   if(rc != 0 && rc != LDAP_SIZELIMIT_EXCEEDED) {
     failf(data, "LDAP remote: %s", ldap_err2string(rc));
@@ -543,14 +544,9 @@
   }
 
   for(i = 0; ludp->lud_attrs && ludp->lud_attrs[i]; i++) {
-    ludp->lud_attrs[i] = curl_easy_unescape(data, ludp->lud_attrs[i], 0, NULL);
-    if(!ludp->lud_attrs[i])
-      return (FALSE);
-  }
-
-  for(i = 0; ludp->lud_exts && ludp->lud_exts[i]; i++) {
-    ludp->lud_exts[i] = curl_easy_unescape(data, ludp->lud_exts[i], 0, NULL);
-    if(!ludp->lud_exts[i])
+    ludp->lud_attrs_dup[i] = curl_easy_unescape(data, ludp->lud_attrs[i],
+                                                0, NULL);
+    if(!ludp->lud_attrs_dup[i])
       return (FALSE);
   }
 
@@ -623,6 +619,11 @@
 
     for(i = 0; ludp->lud_attrs[i]; i++)
       LDAP_TRACE (("attr[%d] '%s'\n", i, ludp->lud_attrs[i]));
+
+    /* allocate the array to receive the unescaped attributes */
+    ludp->lud_attrs_dup = calloc(i+1, sizeof(char*));
+    if(!ludp->lud_attrs_dup)
+      return LDAP_NO_MEMORY;
   }
 
   p = q;
@@ -637,8 +638,9 @@
 
   if(*p && *p != '?') {
     ludp->lud_scope = str2scope(p);
-    if(ludp->lud_scope == -1)
+    if(ludp->lud_scope == -1) {
       return LDAP_INVALID_SYNTAX;
+    }
     LDAP_TRACE (("scope %d\n", ludp->lud_scope));
   }
 
@@ -651,25 +653,13 @@
   q = strchr(p, '?');
   if(q)
     *q++ = '\0';
-  if(!*p)
+  if(!*p) {
     return LDAP_INVALID_SYNTAX;
+  }
 
   ludp->lud_filter = p;
   LDAP_TRACE (("filter '%s'\n", ludp->lud_filter));
 
-  p = q;
-  if(!p)
-    goto success;
-
-  /* parse extensions
-   */
-  ludp->lud_exts = split_str(p);
-  if(!ludp->lud_exts)
-    return LDAP_NO_MEMORY;
-
-  for(i = 0; ludp->lud_exts[i]; i++)
-    LDAP_TRACE (("exts[%d] '%s'\n", i, ludp->lud_exts[i]));
-
   success:
   if(!unescape_elements(conn->data, ludp))
     return LDAP_NO_MEMORY;
@@ -709,16 +699,18 @@
     free(ludp->lud_filter);
 
   if(ludp->lud_attrs) {
-    for(i = 0; ludp->lud_attrs[i]; i++)
-      free(ludp->lud_attrs[i]);
+    if(ludp->lud_attrs_dup) {
+      for(i = 0; ludp->lud_attrs_dup[i]; i++) {
+        if(!ludp->lud_attrs_dup[i])
+          /* abort loop on first NULL */
+          break;
+        free(ludp->lud_attrs_dup[i]);
+      }
+      free(ludp->lud_attrs_dup);
+    }
     free(ludp->lud_attrs);
   }
 
-  if(ludp->lud_exts) {
-    for(i = 0; ludp->lud_exts[i]; i++)
-      free(ludp->lud_exts[i]);
-    free(ludp->lud_exts);
-  }
   free (ludp);
 }
 #endif  /* !HAVE_LDAP_URL_PARSE */