1. openvpn
  2. Commit be16d5f6b050248f503455e4a0e8f3aaaa38bdc7
Browse code

Fix memory leak in argv_extract_cmd_name()

Reported by coverity (in 2009!):

1648 static char *
1649 argv_extract_cmd_name (const char *path)
1650 {
1. Condition path, taking true branch
1651 if (path)
1652 {
1653 char *path_cp = string_alloc(path, NULL); /* POSIX basename()
implementaions may modify its arguments */
1654 const char *bn = basename (path_cp);
2. Condition bn, taking true branch
1655 if (bn)
1656 {
3. alloc_fn: Storage is returned from allocation function
string_alloc. [show details]
4. var_assign: Assigning: ret = storage returned from
string_alloc(bn, NULL).
1657 char *ret = string_alloc (bn, NULL);
5. noescape: Resource ret is not freed or pointed-to in strrchr.
1658 char *dot = strrchr (ret, '.');
6. Condition dot, taking false branch
1659 if (dot)
1660 *dot = '\0';
1661 free(path_cp);
7. Condition ret[0] != 0, taking false branch
1662 if (ret[0] != '\0')
1663 return ret;
CID 27023 (#2-1 of 2): Resource leak (RESOURCE_LEAK)8.
leaked_storage: Variable ret going out of scope leaks the storage it
points to.
1664 }
1665 }
1666 return NULL;
1667 }

This function is only used by argv_printf_arglist(), and in a very specific
case, so it might be that this leak can not even occur. But coverity is
clearly right that this is a bug, so let's just fix it.

Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <1459092130-19905-1-git-send-email-steffan@karger.me>
URL: http://article.gmane.org/gmane.network.openvpn.devel/11369
Signed-off-by: Gert Doering <gert@greenie.muc.de>

Steffan Karger authored on 2016/03/28 00:22:10
Showing 1 changed files
src/openvpn/misc.c
History View file @ be16d5f
... ... 
@@ -1648,22 +1648,27 @@ argv_system_str_append (struct argv *a, const char *str, const bool enquote)
1648 1648 
 static char *
1649 1649 
 argv_extract_cmd_name (const char *path)
1650 1650 
 {
1651 
+  char *ret = NULL;
1651 1652 
   if (path)
1652 1653 
     {
1653 1654 
       char *path_cp = string_alloc(path, NULL); /* POSIX basename() implementaions may modify its arguments */
1654 1655 
       const char *bn = basename (path_cp);
1655 1656 
       if (bn)
1656 1657 
 	{
1657 
-	  char *ret = string_alloc (bn, NULL);
1658 
-	  char *dot = strrchr (ret, '.');
1658 
+	  char *dot = NULL;
1659 
+	  ret = string_alloc (bn, NULL);
1660 
+	  dot = strrchr (ret, '.');
1659 1661 
 	  if (dot)
1660 1662 
 	    *dot = '\0';
1661 1663 
 	  free(path_cp);
1662 
-	  if (ret[0] != '\0')
1663 
-	    return ret;
1664 
+	  if (ret[0] == '\0')
1665 
+	    {
1666 
+	      free(ret);
1667 
+	      ret = NULL;
1668 
+	    }
1664 1669 
 	}
1665 1670 
     }
1666 
-  return NULL;
1671 
+  return ret;
1667 1672 
 }
1668 1673
1669 1674 
 const char *