From db835f8c40e83c6392e69ffc7f2cc500f7682dd4 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemensik@redhat.com>

Date: Fri, 3 Sep 2021 19:23:20 +0200

Subject: [PATCH 10/15] Fix coverity detected issues in option.c

Error: STRING_OVERFLOW (CWE-120): [#def99]

dnsmasq-2.86test7/src/option.c:801: fixed_size_dest: You might overrun the 100-character fixed-size string "buff" by copying "usage[i].arg" without checking the length.

#  799|         if (usage[i].arg)

#  800|   	{

#  801|-> 	  strcpy(buff, usage[i].arg);

#  802|   	  for (j = 0; tab[j].handle; j++)

#  803|   	    if (tab[j].handle == *(usage[i].arg))

Error: CLANG_WARNING: [#def100]

dnsmasq-2.86test7/src/option.c:962:3: warning[deadcode.DeadStores]: Value stored to 'domain' is never read

#  960|       }

#  961|

#  962|->   domain += sprintf(domain, "in-addr.arpa");

#  963|

#  964|     return 1;

Error: CLANG_WARNING: [#def101]

dnsmasq-2.86test7/src/option.c:981:3: warning[deadcode.DeadStores]: Value stored to 'domain' is never read

#  979|         domain += sprintf(domain, "%.1x.", (i>>2) & 1 ? dig & 15 : dig >> 4);

#  980|       }

#  981|->   domain += sprintf(domain, "ip6.arpa");

#  982|

#  983|     return 1;

Error: RESOURCE_LEAK (CWE-772): [#def102] [important]

dnsmasq-2.86test7/src/option.c:1809: alloc_fn: Storage is returned from allocation function "opt_malloc".

dnsmasq-2.86test7/src/option.c:1809: var_assign: Assigning: "path" = storage returned from "opt_malloc(strlen(directory) + len + 2UL)".

dnsmasq-2.86test7/src/option.c:1810: noescape: Resource "path" is not freed or pointed-to in "strcpy". [Note: The source code implementation of the function has been overridden by a builtin model.]

dnsmasq-2.86test7/src/option.c:1811: noescape: Resource "path" is not freed or pointed-to in "strcat". [Note: The source code implementation of the function has been overridden by a builtin model.]

dnsmasq-2.86test7/src/option.c:1812: noescape: Resource "path" is not freed or pointed-to in "strcat". [Note: The source code implementation of the function has been overridden by a builtin model.]

dnsmasq-2.86test7/src/option.c:1815: noescape: Resource "path" is not freed or pointed-to in "stat".

dnsmasq-2.86test7/src/option.c:1809: overwrite_var: Overwriting "path" in "path = opt_malloc(strlen(directory) + len + 2UL)" leaks the storage that "path" points to.

# 1807|   	      continue;

# 1808|

# 1809|-> 	    path = opt_malloc(strlen(directory) + len + 2);

# 1810|   	    strcpy(path, directory);

# 1811|   	    strcat(path, "/");

Error: RESOURCE_LEAK (CWE-772): [#def103] [important]

dnsmasq-2.86test7/src/option.c:1809: alloc_fn: Storage is returned from allocation function "opt_malloc".

dnsmasq-2.86test7/src/option.c:1809: var_assign: Assigning: "path" = storage returned from "opt_malloc(strlen(directory) + len + 2UL)".

dnsmasq-2.86test7/src/option.c:1810: noescape: Resource "path" is not freed or pointed-to in "strcpy". [Note: The source code implementation of the function has been overridden by a builtin model.]

dnsmasq-2.86test7/src/option.c:1811: noescape: Resource "path" is not freed or pointed-to in "strcat". [Note: The source code implementation of the function has been overridden by a builtin model.]

dnsmasq-2.86test7/src/option.c:1812: noescape: Resource "path" is not freed or pointed-to in "strcat". [Note: The source code implementation of the function has been overridden by a builtin model.]

dnsmasq-2.86test7/src/option.c:1815: noescape: Resource "path" is not freed or pointed-to in "stat".

dnsmasq-2.86test7/src/option.c:1858: leaked_storage: Variable "path" going out of scope leaks the storage it points to.

# 1856|   	    free(files);

# 1857|   	  }

# 1858|-> 	break;

# 1859|         }

# 1860|

Error: RESOURCE_LEAK (CWE-772): [#def104] [important]

dnsmasq-2.86test7/src/option.c:1996: alloc_fn: Storage is returned from allocation function "canonicalise_opt".

dnsmasq-2.86test7/src/option.c:1996: var_assign: Assigning: "name" = storage returned from "canonicalise_opt(arg)".

dnsmasq-2.86test7/src/option.c:1998: leaked_storage: Variable "name" going out of scope leaks the storage it points to.

# 1996|   	if (!(name = canonicalise_opt(arg)) ||

# 1997|   	    (comma && !(target = canonicalise_opt(comma))))

# 1998|-> 	  ret_err(_("bad MX name"));

# 1999|

# 2000|   	new = opt_malloc(sizeof(struct mx_srv_record));

Error: RESOURCE_LEAK (CWE-772): [#def106] [important]

dnsmasq-2.86test7/src/option.c:3477: alloc_fn: Storage is returned from allocation function "opt_malloc".

dnsmasq-2.86test7/src/option.c:3477: var_assign: Assigning: "new" = storage returned from "opt_malloc(96UL)".

dnsmasq-2.86test7/src/option.c:3618: leaked_storage: Variable "new" going out of scope leaks the storage it points to.

# 3616|   		      sprintf(errstr, _("duplicate dhcp-host IP address %s"),

# 3617|   			      daemon->addrbuff);

# 3618|-> 		      return 0;

# 3619|   		    }

# 3620|   	      }

Error: RESOURCE_LEAK (CWE-772): [#def108] [important]

dnsmasq-2.86test7/src/option.c:3781: alloc_fn: Storage is returned from allocation function "opt_malloc".

dnsmasq-2.86test7/src/option.c:3781: var_assign: Assigning: "new" = storage returned from "opt_malloc(32UL)".

dnsmasq-2.86test7/src/option.c:3786: leaked_storage: Variable "new" going out of scope leaks the storage it points to.

# 3784|

# 3785|   	if (!(comma = split(arg)) || (len = strlen(comma)) == 0)

# 3786|-> 	  ret_err(gen_err);

# 3787|

# 3788|   	new->wildcard = 0;

Error: RESOURCE_LEAK (CWE-772): [#def109] [important]

dnsmasq-2.86test7/src/option.c:3921: alloc_fn: Storage is returned from allocation function "opt_malloc".

dnsmasq-2.86test7/src/option.c:3921: var_assign: Assigning: "new" = storage returned from "opt_malloc(56UL)".

dnsmasq-2.86test7/src/option.c:3994: leaked_storage: Variable "new" going out of scope leaks the storage it points to.

# 3992|   	   }

# 3993|

# 3994|-> 	 ret_err(gen_err);

# 3995|          }

# 3996|

Error: CLANG_WARNING: [#def111]

dnsmasq-2.86test7/src/option.c:4693:25: warning[deadcode.DeadStores]: Value stored to 'tmp' during its initialization is never read

# 4691|   		if (!canon)

# 4692|                     {

# 4693|-> 		    struct name_list *tmp = new->names, *next;

# 4694|   		    for (tmp = new->names; tmp; tmp = next)

# 4695|

---

 src/option.c | 33 +++++++++++++++++++++------------

 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/option.c b/src/option.c

index ffce9fc..11655fd 100644

--- a/src/option.c

+++ b/src/option.c

@@ -798,7 +798,7 @@ static void do_usage(void)

       if (usage[i].arg)

 	{

-	  strcpy(buff, usage[i].arg);

+	  safe_strncpy(buff, usage[i].arg, sizeof(buff));

 	  for (j = 0; tab[j].handle; j++)

 	    if (tab[j].handle == *(usage[i].arg))

 	      sprintf(buff, "%d", tab[j].val);

@@ -959,7 +959,7 @@ static int domain_rev4(char *domain, struct in_addr addr, int msize)

       return 0;

     }

-  domain += sprintf(domain, "in-addr.arpa");

+  sprintf(domain, "in-addr.arpa");

   return 1;

 }

@@ -978,7 +978,7 @@ static int domain_rev6(char *domain, struct in6_addr *addr, int msize)

       int dig = ((unsigned char *)addr)[i>>3];

       domain += sprintf(domain, "%.1x.", (i>>2) & 1 ? dig & 15 : dig >> 4);

     }

-  domain += sprintf(domain, "ip6.arpa");

+  sprintf(domain, "ip6.arpa");

   return 1;

 }

@@ -1829,6 +1829,8 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma

 		new->next = li;

 		*up = new;

 	      }

+	    else

+	      free(path);

 	  }

@@ -1995,7 +1997,11 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma

 	if (!(name = canonicalise_opt(arg)) || 

 	    (comma && !(target = canonicalise_opt(comma))))

-	  ret_err(_("bad MX name"));

+	  {

+	    free(name);

+	    free(target);

+	    ret_err(_("bad MX name"));

+	  }

 	new = opt_malloc(sizeof(struct mx_srv_record));

 	new->next = daemon->mxnames;

@@ -3616,6 +3622,7 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma

 		      inet_ntop(AF_INET, &in, daemon->addrbuff, ADDRSTRLEN);

 		      sprintf(errstr, _("duplicate dhcp-host IP address %s"),

 			      daemon->addrbuff);

+		      dhcp_config_free(new);

 		      return 0;

 		    }	      

 	      }

@@ -3779,16 +3786,16 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma

     case LOPT_NAME_MATCH: /* --dhcp-name-match */

       {

-	struct dhcp_match_name *new = opt_malloc(sizeof(struct dhcp_match_name));

-	struct dhcp_netid *id = opt_malloc(sizeof(struct dhcp_netid));

+	struct dhcp_match_name *new;

 	ssize_t len;

 	if (!(comma = split(arg)) || (len = strlen(comma)) == 0)

 	  ret_err(gen_err);

+	new = opt_malloc(sizeof(struct dhcp_match_name));

 	new->wildcard = 0;

-	new->netid = id;

-	id->net = opt_string_alloc(set_prefix(arg));

+	new->netid = opt_malloc(sizeof(struct dhcp_netid));

+	new->netid->net = opt_string_alloc(set_prefix(arg));

 	if (comma[len-1] == '*')

 	  {

@@ -3992,6 +3999,8 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma

 	       }

 	   }

+	 dhcp_netid_free(new->netid);

+	 free(new);

 	 ret_err(gen_err);

        }

@@ -4367,7 +4376,7 @@ err:

     case LOPT_CNAME: /* --cname */

       {

 	struct cname *new;

-	char *alias, *target, *last, *pen;

+	char *alias, *target=NULL, *last, *pen;

 	int ttl = -1;

 	for (last = pen = NULL, comma = arg; comma; comma = split(comma))

@@ -4382,13 +4391,13 @@ err:

 	if (pen != arg && atoi_check(last, &ttl))

 	  last = pen;

-    	target = canonicalise_opt(last);

-

 	while (arg != last)

 	  {

 	    int arglen = strlen(arg);

 	    alias = canonicalise_opt(arg);

+	    if (!target)

+	      target = canonicalise_opt(last);

 	    if (!alias || !target)

 	      {

 		free(target);

@@ -4691,7 +4700,7 @@ err:

 		struct name_list *nl;

 		if (!canon)

                   {

-		    struct name_list *tmp = new->names, *next;

+		    struct name_list *tmp, *next;

 		    for (tmp = new->names; tmp; tmp = next)

 		      {

 			next = tmp->next;

-- 

2.31.1