From db835f8c40e83c6392e69ffc7f2cc500f7682dd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= 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