]> xenbits.xensource.com Git - people/julieng/freebsd.git/commitdiff
Fix several memory leaks, and crashes, in iconvlist(3).
authorbdrewery <bdrewery@FreeBSD.org>
Thu, 29 Oct 2015 23:02:34 +0000 (23:02 +0000)
committerbdrewery <bdrewery@FreeBSD.org>
Thu, 29 Oct 2015 23:02:34 +0000 (23:02 +0000)
- Both curitem and curitem (via the names list) was always leaked.
- malloc(3) failures lead to some leaks.
- __bsd___iconv_get_list() failure lead to a crash since its error was not
  handles and __bsd___iconv_free_list() is not NULL-safe.

I have slightly refactored this to avoid extra malloc and free logic in cases
of malloc(3) failing.

There are still bad assumptions here that I did not deal with.  One of which is
that the data will always have a '/' so the strchr(3) will not return NULL.

Coverity CID: 1130055 1130054 1130053

lib/libc/iconv/bsd_iconv.c

index e032a5b1e0b23560af9674cc07ef2225abb8f87a..e9ff688887c98ce53eccab15a09e50bfee32fa08 100644 (file)
@@ -207,43 +207,51 @@ __bsd_iconvlist(int (*do_one) (unsigned int, const char * const *,
        const char * const *np;
        char *curitem, *curkey, *slashpos;
        size_t sz;
-       unsigned int i, j;
+       unsigned int i, j, n;
 
        i = 0;
+       names = NULL;
 
-       if (__bsd___iconv_get_list(&list, &sz, true))
+       if (__bsd___iconv_get_list(&list, &sz, true)) {
                list = NULL;
+               goto out;
+       }
        qsort((void *)list, sz, sizeof(char *), qsort_helper);
        while (i < sz) {
                j = 0;
                slashpos = strchr(list[i], '/');
-               curkey = (char *)malloc(slashpos - list[i] + 2);
-               names = (char **)malloc(sz * sizeof(char *));
-               if ((curkey == NULL) || (names == NULL)) {
-                       __bsd___iconv_free_list(list, sz);
-                       return;
-               }
-               strlcpy(curkey, list[i], slashpos - list[i] + 1);
+               names = malloc(sz * sizeof(char *));
+               if (names == NULL)
+                       goto out;
+               curkey = strndup(list[i], slashpos - list[i]);
+               if (curkey == NULL)
+                       goto out;
                names[j++] = curkey;
                for (; (i < sz) && (memcmp(curkey, list[i], strlen(curkey)) == 0); i++) {
                        slashpos = strchr(list[i], '/');
-                       curitem = (char *)malloc(strlen(slashpos) + 1);
-                       if (curitem == NULL) {
-                               __bsd___iconv_free_list(list, sz);
-                               return;
-                       }
-                       strlcpy(curitem, &slashpos[1], strlen(slashpos) + 1);
-                       if (strcmp(curkey, curitem) == 0) {
+                       if (strcmp(curkey, &slashpos[1]) == 0)
                                continue;
-                       }
+                       curitem = strdup(&slashpos[1]);
+                       if (curitem == NULL)
+                               goto out;
                        names[j++] = curitem;
                }
                np = (const char * const *)names;
                do_one(j, np, data);
+               for (n = 0; n < j; n++)
+                       free(names[n]);
                free(names);
+               names = NULL;
        }
 
-       __bsd___iconv_free_list(list, sz);
+out:
+       if (names != NULL) {
+               for (n = 0; n < j; n++)
+                       free(names[n]);
+               free(names);
+       }
+       if (list != NULL)
+               __bsd___iconv_free_list(list, sz);
 }
 
 __inline const char *