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.

---
 src/openvpn/misc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

index 05ed073..f76c2e5 100644
@@ -1648,22 +1648,27 @@ argv_system_str_append (struct argv *a, const char *str, const bool enquote)
 static char *
 argv_extract_cmd_name (const char *path)
 {
+  char *ret = NULL;
   if (path)
     {
       char *path_cp = string_alloc(path, NULL); /* POSIX basename() implementaions may modify its arguments */
       const char *bn = basename (path_cp);
       if (bn)
 	{
-	  char *ret = string_alloc (bn, NULL);
-	  char *dot = strrchr (ret, '.');
+	  char *dot = NULL;
+	  ret = string_alloc (bn, NULL);
+	  dot = strrchr (ret, '.');
 	  if (dot)
 	    *dot = '\0';
 	  free(path_cp);
-	  if (ret[0] != '\0')
-	    return ret;
+	  if (ret[0] == '\0')
+	    {
+	      free(ret);
+	      ret = NULL;
+	    }
 	}
     }
-  return NULL;
+  return ret;
 }
 
 const char *

2.5.0

ACK. (The code in question is today only ever called from a context where it is not leaking, but then, we might change this one day...)

Your patch has been applied to the master and release/2.3 branch.

commit be16d5f6b050248f503455e4a0e8f3aaaa38bdc7 (master) commit f8da74920998a8fa1125ec73c5e3397707b43017 (release/2.3)

Author: Steffan Karger
Date:   Sun Mar 27 17:22:10 2016 +0200

     Fix memory leak in argv_extract_cmd_name()

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/11369
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering