svn commit: r290168 - head/lib/libc/iconv

[Bryan Drewery]

Author: bdrewery
Date: Thu Oct 29 23:02:34 2015
New Revision: 290168
URL: https://svnweb.freebsd.org/changeset/base/290168

Log:
  Fix several memory leaks, and crashes, in iconvlist(3).
  
  - 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

Modified:
  head/lib/libc/iconv/bsd_iconv.c

Modified: head/lib/libc/iconv/bsd_iconv.c
==============================================================================
--- head/lib/libc/iconv/bsd_iconv.c	Thu Oct 29 22:12:03 2015	(r290167)
+++ head/lib/libc/iconv/bsd_iconv.c	Thu Oct 29 23:02:34 2015	(r290168)
@@ -207,43 +207,51 @@ __bsd_iconvlist(int (*do_one) (unsigned 
 	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 *