237 lines
9.8 KiB
Diff
237 lines
9.8 KiB
Diff
|
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
|
||
|
|