Notification time stamped 2021-09-09 08:04:29 UTC
From 06b5c95b058e4985a844a72bfc98ab0dd157a364 Mon Sep 17 00:00:00 2001 From: Petr Menšík pemensik@redhat.com Date: Sep 09 2021 07:55:19 +0000 Subject: Add coverity patches
Various coverity fixes, not yet sent to upstream.
---
diff --git a/0001-Retry-on-interrupted-error-in-tftp.patch b/0001-Retry-on-interrupted-error-in-tftp.patch new file mode 100644 index 0000000..f486f2d --- /dev/null +++ b/0001-Retry-on-interrupted-error-in-tftp.patch @@ -0,0 +1,35 @@ +From f5f56c001dddd486859dc6301e6cbe00ba604fe8 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= pemensik@redhat.com +Date: Wed, 18 Aug 2021 10:09:35 +0200 +Subject: [PATCH 01/15] Retry on interrupted error in tftp + +Interrupt might arrive when sending error reply. Retry if possible. + +Wrong Check of Return Value + +10. dnsmasq-2.85/src/tftp.c:603: check_return: Calling "sendto(transfer->sockfd, dnsmasq_daemon->packet, len, 0, __CONST_SOCKADDR_ARG({.__sockaddr__ = &peer.sa}), sa_len(&peer))" without checking return value. This library function may fail and return an error code. + # 601| prettyprint_addr(&peer, daemon->addrbuff); + # 602| len = tftp_err(ERR_TID, daemon->packet, _("ignoring packet from %s (TID mismatch)"), daemon->addrbuff); + # 603|-> sendto(transfer->sockfd, daemon->packet, len, 0, &peer.sa, sa_len(&peer)); + # 604| } + # 605| } +--- + src/tftp.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/tftp.c b/src/tftp.c +index 37bdff2..3d87523 100644 +--- a/src/tftp.c ++++ b/src/tftp.c +@@ -600,7 +600,7 @@ void check_tftp_listeners(time_t now) + /* Wrong source address. See rfc1350 para 4. */ + prettyprint_addr(&peer, daemon->addrbuff); + len = tftp_err(ERR_TID, daemon->packet, _("ignoring packet from %s (TID mismatch)"), daemon->addrbuff); +- sendto(transfer->sockfd, daemon->packet, len, 0, &peer.sa, sa_len(&peer)); ++ while(retry_send(sendto(transfer->sockfd, daemon->packet, len, 0, &peer.sa, sa_len(&peer)))); + } + } + } +-- +2.31.1 + diff --git a/0002-Add-safety-checks-to-places-pointed-by-Coverity.patch b/0002-Add-safety-checks-to-places-pointed-by-Coverity.patch new file mode 100644 index 0000000..8fc70a5 --- /dev/null +++ b/0002-Add-safety-checks-to-places-pointed-by-Coverity.patch @@ -0,0 +1,77 @@ +From 061013293ceddce509ae06a31a045e803103f1ce Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= pemensik@redhat.com +Date: Wed, 18 Aug 2021 14:59:23 +0200 +Subject: [PATCH 02/15] Add safety checks to places pointed by Coverity + +GCC Analyzer (experimental) + +1. dnsmasq-2.85/src/forward.c:0: scope_hint: In function 'allocate_rfd.part.0' +2. dnsmasq-2.85/src/forward.c:2321:18: warning[-Wanalyzer-null-dereference]: dereference of NULL 'rfd' + # 2319| *fdlp = rfl; + # 2320| + # 2321|-> return rfl->rfd->fd; + # 2322| } + # 2323| + +1. dnsmasq-2.85/src/cache.c:0: scope_hint: In function 'log_query' +2. dnsmasq-2.85/src/cache.c:1969:20: warning[-Wanalyzer-null-dereference]: dereference of NULL 'name' + # 1967| source = "cached"; + # 1968| + # 1969|-> if (strlen(name) == 0) + # 1970| name = "."; + # 1971| + +1. dnsmasq-2.85/src/cache.c:0: scope_hint: In function 'cache_scan_free' +2. dnsmasq-2.85/src/cache.c:436:20: warning[-Wanalyzer-null-argument]: use of NULL 'addr' where non-null expected +40. /usr/include/sys/un.h:37: included_from: Included from here. +41. dnsmasq-2.85/src/dnsmasq.h:101: included_from: Included from here. +42. dnsmasq-2.85/src/cache.c:17: included_from: Included from here. +43. /usr/include/string.h:64:12: note: argument 2 of 'memcmp' must be non-null + # 434| (flags & crecp->flags & F_REVERSE) && + # 435| (flags & crecp->flags & (F_IPV4 | F_IPV6)) && + # 436|-> memcmp(&crecp->addr, addr, addrlen) == 0) + # 437| { + # 438| *up = crecp->hash_next; +--- + src/cache.c | 4 ++-- + src/forward.c | 2 +- + 2 files changed, 3 insertions(+), 3 deletions(-) + +diff --git a/src/cache.c b/src/cache.c +index 8add610..97c51a7 100644 +--- a/src/cache.c ++++ b/src/cache.c +@@ -433,7 +433,7 @@ static struct crec *cache_scan_free(char *name, union all_addr *addr, unsigned s + else if (!(crecp->flags & (F_HOSTS | F_DHCP | F_CONFIG)) && + (flags & crecp->flags & F_REVERSE) && + (flags & crecp->flags & (F_IPV4 | F_IPV6)) && +- memcmp(&crecp->addr, addr, addrlen) == 0) ++ addr && memcmp(&crecp->addr, addr, addrlen) == 0) + { + *up = crecp->hash_next; + cache_unlink(crecp); +@@ -2013,7 +2013,7 @@ void log_query(unsigned int flags, char *name, union all_addr *addr, char *arg) + else + source = "cached"; + +- if (strlen(name) == 0) ++ if (name && !name[0]) + name = "."; + + if (option_bool(OPT_EXTRALOG)) +diff --git a/src/forward.c b/src/forward.c +index 3d638e4..f07c908 100644 +--- a/src/forward.c ++++ b/src/forward.c +@@ -2276,7 +2276,7 @@ int allocate_rfd(struct randfd_list **fdlp, struct server *serv) + } + } + +- if (j == daemon->numrrand) ++ if (!rfd) /* should be when j == daemon->numrrand */ + { + struct randfd_list *rfl_poll; + +-- +2.31.1 + diff --git a/0003-Small-safeguard-to-unexpected-data.patch b/0003-Small-safeguard-to-unexpected-data.patch new file mode 100644 index 0000000..c496640 --- /dev/null +++ b/0003-Small-safeguard-to-unexpected-data.patch @@ -0,0 +1,28 @@ +From 920cd815bafea084f68cc4309399aea77bd7f66b 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 14:11:42 +0200 +Subject: [PATCH 03/15] Small safeguard to unexpected data + +Make sure negative index is not used for comparison. It seems code in +option parsing does not allow it to be empty, but insist on it also in +this place. +--- + src/dhcp-common.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/dhcp-common.c b/src/dhcp-common.c +index 73568a9..85b269a 100644 +--- a/src/dhcp-common.c ++++ b/src/dhcp-common.c +@@ -88,7 +88,7 @@ int match_netid_wild(struct dhcp_netid *check, struct dhcp_netid *pool) + for (; check; check = check->next) + { + const int check_len = strlen(check->net); +- const int is_wc = (check->net[check_len - 1] == '*'); ++ const int is_wc = (check_len > 0 && check->net[check_len - 1] == '*'); + + /* '#' for not is for backwards compat. */ + if (check->net[0] != '!' && check->net[0] != '#') +-- +2.31.1 + diff --git a/0004-Fix-bunch-of-warnings-in-auth.c.patch b/0004-Fix-bunch-of-warnings-in-auth.c.patch new file mode 100644 index 0000000..976255f --- /dev/null +++ b/0004-Fix-bunch-of-warnings-in-auth.c.patch @@ -0,0 +1,153 @@ +From e61af561900b4d2dd976a575b2efd388be092742 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 16:00:35 +0200 +Subject: [PATCH 04/15] Fix bunch of warnings in auth.c + +Error: CLANG_WARNING: [#def7] +dnsmasq-2.86test7/src/auth.c:420:5: warning[deadcode.DeadStores]: Value stored to 'found' is never read + # 418| if (!found && is_name_synthetic(flag, name, &addr) ) + # 419| { + # 420|-> found = 1; + # 421| nxdomain = 0; + # 422| + +Error: CLANG_WARNING: [#def8] +dnsmasq-2.86test7/src/auth.c:436:8: warning[deadcode.DeadStores]: Value stored to 'found' is never read + # 434| { + # 435| auth = soa = 1; /* inhibits auth section */ + # 436|-> found = 1; + # 437| log_query(F_RRNAME | F_AUTH, zone->domain, NULL, "<SOA>"); + # 438| } + +Error: CLANG_WARNING: [#def9] +dnsmasq-2.86test7/src/auth.c:472:8: warning[deadcode.DeadStores]: Value stored to 'found' is never read + # 470| ns = 1; /* ensure we include NS records! */ + # 471| axfr = 1; + # 472|-> found = 1; + # 473| axfroffset = nameoffset; + # 474| log_query(F_RRNAME | F_AUTH, zone->domain, NULL, "<AXFR>"); + +Error: CLANG_WARNING: [#def10] +dnsmasq-2.86test7/src/auth.c:480:8: warning[deadcode.DeadStores]: Value stored to 'found' is never read + # 478| auth = 1; + # 479| ns = 1; /* inhibits auth section */ + # 480|-> found = 1; + # 481| log_query(F_RRNAME | F_AUTH, zone->domain, NULL, "<NS>"); + # 482| } + +Error: CLANG_WARNING: [#def11] +dnsmasq-2.86test7/src/auth.c:501:4: warning[deadcode.DeadStores]: Value stored to 'found' is never read + # 499| log_query(crecp->flags, name, &crecp->addr, record_source(crecp->uid)); + # 500| *cut = 0; /* remove domain part */ + # 501|-> found = 1; + # 502| if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, + # 503| daemon->auth_ttl, NULL, qtype, C_IN, + +Error: CLANG_WARNING: [#def12] +dnsmasq-2.86test7/src/auth.c:522:8: warning[deadcode.DeadStores]: Value stored to 'found' is never read + # 520| { + # 521| log_query(crecp->flags, name, &crecp->addr, record_source(crecp->uid)); + # 522|-> found = 1; + # 523| if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, + # 524| daemon->auth_ttl, NULL, qtype, C_IN, + +Error: CLANG_WARNING: [#def13] +dnsmasq-2.86test7/src/auth.c:617:8: warning[deadcode.DeadStores]: Value stored to 'p' is never read + # 615| p += sprintf(p, "%u.", a & 0xff); + # 616| a = a >> 8; + # 617|-> p += sprintf(p, "%u.in-addr.arpa", a & 0xff); + # 618| + # 619| } + +Error: CPPCHECK_WARNING (CWE-758): [#def14] +dnsmasq-2.86test7/src/auth.c:627: warning[objectIndex]: The address of local variable 'addr6' might be accessed at non-zero index. + # 625| for (i = subnet->prefixlen-1; i >= 0; i -= 4) + # 626| { + # 627|-> int dig = ((unsigned char *)&subnet->addr.addr6)[i>>3]; + # 628| p += sprintf(p, "%.1x.", (i>>2) & 1 ? dig & 15 : dig >> 4); + # 629| } + +Error: CLANG_WARNING: [#def15] +dnsmasq-2.86test7/src/auth.c:630:8: warning[deadcode.DeadStores]: Value stored to 'p' is never read + # 628| p += sprintf(p, "%.1x.", (i>>2) & 1 ? dig & 15 : dig >> 4); + # 629| } + # 630|-> p += sprintf(p, "ip6.arpa"); + # 631| + # 632| } +--- + src/auth.c | 10 ++-------- + 1 file changed, 2 insertions(+), 8 deletions(-) + +diff --git a/src/auth.c b/src/auth.c +index 172a4b2..4f03c39 100644 +--- a/src/auth.c ++++ b/src/auth.c +@@ -417,7 +417,6 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n + + if (!found && is_name_synthetic(flag, name, &addr) ) + { +- found = 1; + nxdomain = 0; + + log_query(F_FORWARD | F_CONFIG | flag, name, &addr, NULL); +@@ -433,7 +432,6 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n + if (qtype == T_SOA) + { + auth = soa = 1; /* inhibits auth section */ +- found = 1; + log_query(F_RRNAME | F_AUTH, zone->domain, NULL, "<SOA>"); + } + else if (qtype == T_AXFR) +@@ -469,7 +467,6 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n + soa = 1; /* inhibits auth section */ + ns = 1; /* ensure we include NS records! */ + axfr = 1; +- found = 1; + axfroffset = nameoffset; + log_query(F_RRNAME | F_AUTH, zone->domain, NULL, "<AXFR>"); + } +@@ -477,7 +474,6 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n + { + auth = 1; + ns = 1; /* inhibits auth section */ +- found = 1; + log_query(F_RRNAME | F_AUTH, zone->domain, NULL, "<NS>"); + } + } +@@ -498,7 +494,6 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n + *cut = '.'; /* restore domain part */ + log_query(crecp->flags, name, &crecp->addr, record_source(crecp->uid)); + *cut = 0; /* remove domain part */ +- found = 1; + if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, + daemon->auth_ttl, NULL, qtype, C_IN, + qtype == T_A ? "4" : "6", &crecp->addr)) +@@ -519,7 +514,6 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n + if ((crecp->flags & flag) && (local_query || filter_zone(zone, flag, &(crecp->addr)))) + { + log_query(crecp->flags, name, &crecp->addr, record_source(crecp->uid)); +- found = 1; + if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, + daemon->auth_ttl, NULL, qtype, C_IN, + qtype == T_A ? "4" : "6", &crecp->addr)) +@@ -614,7 +608,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n + if (subnet->prefixlen >= 16 ) + p += sprintf(p, "%u.", a & 0xff); + a = a >> 8; +- p += sprintf(p, "%u.in-addr.arpa", a & 0xff); ++ sprintf(p, "%u.in-addr.arpa", a & 0xff); + + } + else +@@ -627,7 +621,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n + int dig = ((unsigned char *)&subnet->addr.addr6)[i>>3]; + p += sprintf(p, "%.1x.", (i>>2) & 1 ? dig & 15 : dig >> 4); + } +- p += sprintf(p, "ip6.arpa"); ++ sprintf(p, "ip6.arpa"); + + } + } +-- +2.31.1 + diff --git a/0005-Fix-few-coverity-warnings-in-lease-tools.patch b/0005-Fix-few-coverity-warnings-in-lease-tools.patch new file mode 100644 index 0000000..ab24440 --- /dev/null +++ b/0005-Fix-few-coverity-warnings-in-lease-tools.patch @@ -0,0 +1,148 @@ +From be7f213066282baeed46cc34223601c462db9cbf 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 16:32:05 +0200 +Subject: [PATCH 05/15] Fix few coverity warnings in lease-tools + +Error: UNINIT (CWE-457): [#def2] +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release.c:265: var_decl: Declaring variable "ifr" without initializer. +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release.c:285: uninit_use_in_call: Using uninitialized value "ifr". Field "ifr.ifr_ifru" is uninitialized when calling "setsockopt". + # 283| strncpy(ifr.ifr_name, argv[1], sizeof(ifr.ifr_name)-1); + # 284| ifr.ifr_name[sizeof(ifr.ifr_name)-1] = '\0'; + # 285|-> if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, &ifr, sizeof(ifr)) == -1) + # 286| { + # 287| perror("cannot setup interface"); + +Error: CHECKED_RETURN (CWE-252): [#def3] +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release6.c:346: check_return: Calling "inet_pton" without checking return value (as is done elsewhere 61 out of 72 times). +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release6.c:188: example_assign: Example 1: Assigning: "s" = return value from "inet_pton(10, ip, &result.ip)". +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release6.c:189: example_checked: Example 1 (cont.): "s" has its value checked in "s <= 0". +dnsmasq-2.86test7/src/cache.c:1108: example_checked: Example 2: "inet_pton(10, token, &addr)" has its value checked in "inet_pton(10, token, &addr) > 0". +dnsmasq-2.86test7/src/dbus.c:525: example_checked: Example 3: "inet_pton(2, ipaddr, &addr.addr4)" has its value checked in "inet_pton(2, ipaddr, &addr.addr4)". +dnsmasq-2.86test7/src/domain.c:138: example_checked: Example 4: "inet_pton(prot, tail, addr)" has its value checked in "inet_pton(prot, tail, addr)". +dnsmasq-2.86test7/src/lease.c:81: example_checked: Example 5: "inet_pton(10, dnsmasq_daemon->namebuff, &addr.addr6)" has its value checked in "inet_pton(10, dnsmasq_daemon->namebuff, &addr.addr6)". + # 344| client_addr.sin6_flowinfo = 0; + # 345| client_addr.sin6_scope_id =0; + # 346|-> inet_pton(AF_INET6, "::", &client_addr.sin6_addr); + # 347| bind(sock, (struct sockaddr*)&client_addr, sizeof(struct sockaddr_in6)); + # 348| inet_pton(AF_INET6, DHCP6_MULTICAST_ADDRESS, &server_addr.sin6_addr); + +Error: CHECKED_RETURN (CWE-252): [#def4] +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release6.c:347: check_return: Calling "bind(sock, (struct sockaddr *)&client_addr, 28U)" without checking return value. This library function may fail and return an error code. + # 345| client_addr.sin6_scope_id =0; + # 346| inet_pton(AF_INET6, "::", &client_addr.sin6_addr); + # 347|-> bind(sock, (struct sockaddr*)&client_addr, sizeof(struct sockaddr_in6)); + # 348| inet_pton(AF_INET6, DHCP6_MULTICAST_ADDRESS, &server_addr.sin6_addr); + # 349| server_addr.sin6_port = htons(DHCP6_SERVER_PORT); + +Error: CHECKED_RETURN (CWE-252): [#def5] +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release6.c:348: check_return: Calling "inet_pton" without checking return value (as is done elsewhere 61 out of 72 times). +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release6.c:188: example_assign: Example 1: Assigning: "s" = return value from "inet_pton(10, ip, &result.ip)". +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release6.c:189: example_checked: Example 1 (cont.): "s" has its value checked in "s <= 0". +dnsmasq-2.86test7/src/cache.c:1108: example_checked: Example 2: "inet_pton(10, token, &addr)" has its value checked in "inet_pton(10, token, &addr) > 0". +dnsmasq-2.86test7/src/dbus.c:525: example_checked: Example 3: "inet_pton(2, ipaddr, &addr.addr4)" has its value checked in "inet_pton(2, ipaddr, &addr.addr4)". +dnsmasq-2.86test7/src/domain.c:138: example_checked: Example 4: "inet_pton(prot, tail, addr)" has its value checked in "inet_pton(prot, tail, addr)". +dnsmasq-2.86test7/src/lease.c:81: example_checked: Example 5: "inet_pton(10, dnsmasq_daemon->namebuff, &addr.addr6)" has its value checked in "inet_pton(10, dnsmasq_daemon->namebuff, &addr.addr6)". + # 346| inet_pton(AF_INET6, "::", &client_addr.sin6_addr); + # 347| bind(sock, (struct sockaddr*)&client_addr, sizeof(struct sockaddr_in6)); + # 348|-> inet_pton(AF_INET6, DHCP6_MULTICAST_ADDRESS, &server_addr.sin6_addr); + # 349| server_addr.sin6_port = htons(DHCP6_SERVER_PORT); + # 350| int16_t recv_size = 0; + +Error: NEGATIVE_RETURNS (CWE-394): [#def6] +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release6.c:360: var_tested_neg: Variable "recv_size" tests negative. +dnsmasq-2.86test7/contrib/lease-tools/dhcp_release6.c:373: negative_returns: "recv_size" is passed to a parameter that cannot be negative. + # 371| } + # 372| + # 373|-> int16_t result = parse_packet(response, recv_size); + # 374| if (result == NOT_REPLY_CODE) + # 375| { +--- + contrib/lease-tools/dhcp_release.c | 1 + + contrib/lease-tools/dhcp_release6.c | 37 ++++++++++++++++++----------- + 2 files changed, 24 insertions(+), 14 deletions(-) + +diff --git a/contrib/lease-tools/dhcp_release.c b/contrib/lease-tools/dhcp_release.c +index c1c835b..84f5610 100644 +--- a/contrib/lease-tools/dhcp_release.c ++++ b/contrib/lease-tools/dhcp_release.c +@@ -280,6 +280,7 @@ int main(int argc, char **argv) + + /* This voodoo fakes up a packet coming from the correct interface, which really matters for + a DHCP server */ ++ memset(&ifr, 0, sizeof(ifr)); + strncpy(ifr.ifr_name, argv[1], sizeof(ifr.ifr_name)-1); + ifr.ifr_name[sizeof(ifr.ifr_name)-1] = '\0'; + if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, &ifr, sizeof(ifr)) == -1) +diff --git a/contrib/lease-tools/dhcp_release6.c b/contrib/lease-tools/dhcp_release6.c +index d680222..9b3438f 100644 +--- a/contrib/lease-tools/dhcp_release6.c ++++ b/contrib/lease-tools/dhcp_release6.c +@@ -318,6 +318,12 @@ void usage(const char* arg, FILE* stream) + fprintf (stream, "Usage: %s %s\n", arg, usage_string); + } + ++static void fail_fatal(const char *errstr, int exitcode) ++{ ++ perror(errstr); ++ exit(exitcode); ++} ++ + int send_release_packet(const char* iface, struct dhcp6_packet* packet) + { + struct sockaddr_in6 server_addr, client_addr; +@@ -343,18 +349,19 @@ int send_release_packet(const char* iface, struct dhcp6_packet* packet) + client_addr.sin6_port = htons(DHCP6_CLIENT_PORT); + client_addr.sin6_flowinfo = 0; + client_addr.sin6_scope_id =0; +- inet_pton(AF_INET6, "::", &client_addr.sin6_addr); +- bind(sock, (struct sockaddr*)&client_addr, sizeof(struct sockaddr_in6)); +- inet_pton(AF_INET6, DHCP6_MULTICAST_ADDRESS, &server_addr.sin6_addr); ++ if (inet_pton(AF_INET6, "::", &client_addr.sin6_addr) <= 0) ++ fail_fatal("inet_pton", 5); ++ if (bind(sock, (struct sockaddr*)&client_addr, sizeof(struct sockaddr_in6)) != 0) ++ perror("bind"); /* continue on bind error */ ++ if (inet_pton(AF_INET6, DHCP6_MULTICAST_ADDRESS, &server_addr.sin6_addr) <= 0) ++ fail_fatal("inet_pton", 5); + server_addr.sin6_port = htons(DHCP6_SERVER_PORT); +- int16_t recv_size = 0; ++ ssize_t recv_size = 0; ++ int result; + for (i = 0; i < 5; i++) + { + if (sendto(sock, packet->buf, packet->len, 0, (struct sockaddr *)&server_addr, sizeof(server_addr)) < 0) +- { +- perror("sendto failed"); +- exit(4); +- } ++ fail_fatal("sendto failed", 4); + + recv_size = recvfrom(sock, response, sizeof(response), MSG_DONTWAIT, NULL, 0); + if (recv_size == -1) +@@ -367,16 +374,18 @@ int send_release_packet(const char* iface, struct dhcp6_packet* packet) + else + { + perror("recvfrom"); ++ result = UNSPEC_FAIL; + } + } +- +- int16_t result = parse_packet(response, recv_size); +- if (result == NOT_REPLY_CODE) ++ else + { +- sleep(1); +- continue; ++ result = parse_packet(response, recv_size); ++ if (result == NOT_REPLY_CODE) ++ { ++ sleep(1); ++ continue; ++ } + } +- + close(sock); + return result; + } +-- +2.31.1 + diff --git a/0006-Fix-coverity-formats-issues-in-blockdata.patch b/0006-Fix-coverity-formats-issues-in-blockdata.patch new file mode 100644 index 0000000..beb0898 --- /dev/null +++ b/0006-Fix-coverity-formats-issues-in-blockdata.patch @@ -0,0 +1,48 @@ +From 3a077065ce846e301b532127ebecdd2771ad75ed 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 16:41:00 +0200 +Subject: [PATCH 06/15] Fix coverity formats issues in blockdata + +Error: PRINTF_ARGS (CWE-686): [#def16] +dnsmasq-2.86test7/src/blockdata.c:56: invalid_type: Argument "blockdata_count * 48UL" to format specifier "%u" was expected to have type "unsigned int" but has type "unsigned long". + # 54| { + # 55| my_syslog(LOG_INFO, _("pool memory in use %u, max %u, allocated %u"), + # 56|-> blockdata_count * sizeof(struct blockdata), + # 57| blockdata_hwm * sizeof(struct blockdata), + # 58| blockdata_alloced * sizeof(struct blockdata)); + +Error: PRINTF_ARGS (CWE-686): [#def17] +dnsmasq-2.86test7/src/blockdata.c:57: invalid_type: Argument "blockdata_hwm * 48UL" to format specifier "%u" was expected to have type "unsigned int" but has type "unsigned long". + # 55| my_syslog(LOG_INFO, _("pool memory in use %u, max %u, allocated %u"), + # 56| blockdata_count * sizeof(struct blockdata), + # 57|-> blockdata_hwm * sizeof(struct blockdata), + # 58| blockdata_alloced * sizeof(struct blockdata)); + # 59| } + +Error: PRINTF_ARGS (CWE-686): [#def18] +dnsmasq-2.86test7/src/blockdata.c:58: invalid_type: Argument "blockdata_alloced * 48UL" to format specifier "%u" was expected to have type "unsigned int" but has type "unsigned long". + # 56| blockdata_count * sizeof(struct blockdata), + # 57| blockdata_hwm * sizeof(struct blockdata), + # 58|-> blockdata_alloced * sizeof(struct blockdata)); + # 59| } + # 60| +--- + src/blockdata.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/blockdata.c b/src/blockdata.c +index f7740b5..0986285 100644 +--- a/src/blockdata.c ++++ b/src/blockdata.c +@@ -52,7 +52,7 @@ void blockdata_init(void) + + void blockdata_report(void) + { +- my_syslog(LOG_INFO, _("pool memory in use %u, max %u, allocated %u"), ++ my_syslog(LOG_INFO, _("pool memory in use %zu, max %zu, allocated %zu"), + blockdata_count * sizeof(struct blockdata), + blockdata_hwm * sizeof(struct blockdata), + blockdata_alloced * sizeof(struct blockdata)); +-- +2.31.1 + diff --git a/0007-Retry-dhcp6-ping-on-interrupts.patch b/0007-Retry-dhcp6-ping-on-interrupts.patch new file mode 100644 index 0000000..1beb806 --- /dev/null +++ b/0007-Retry-dhcp6-ping-on-interrupts.patch @@ -0,0 +1,32 @@ +From 467b621fb7da6e1318ac7204325b0adb01b3ff19 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 16:48:50 +0200 +Subject: [PATCH 07/15] Retry dhcp6 ping on interrupts + +Error: CHECKED_RETURN (CWE-252): [#def35] +dnsmasq-2.86test7/src/dhcp6.c:295: check_return: Calling "sendto(dnsmasq_daemon->icmp6fd, &neigh, 24UL, 0, __CONST_SOCKADDR_ARG({.__sockaddr__ = &addr.sa}), 28U)" without checking return value. This library function may fail and return an error code. + # 293| break; + # 294| + # 295|-> sendto(daemon->icmp6fd, &neigh, sizeof(neigh), 0, &addr.sa, sizeof(addr)); + # 296| + # 297| ts.tv_sec = 0; +--- + src/dhcp6.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/dhcp6.c b/src/dhcp6.c +index 2be877f..ae1f5c1 100644 +--- a/src/dhcp6.c ++++ b/src/dhcp6.c +@@ -292,7 +292,7 @@ void get_client_mac(struct in6_addr *client, int iface, unsigned char *mac, unsi + if ((maclen = find_mac(&addr, mac, 0, now)) != 0) + break; + +- sendto(daemon->icmp6fd, &neigh, sizeof(neigh), 0, &addr.sa, sizeof(addr)); ++ while(retry_send(sendto(daemon->icmp6fd, &neigh, sizeof(neigh), 0, &addr.sa, sizeof(addr)))); + + ts.tv_sec = 0; + ts.tv_nsec = 100000000; /* 100ms */ +-- +2.31.1 + diff --git a/0008-Fix-coverity-warnings-on-dbus.patch b/0008-Fix-coverity-warnings-on-dbus.patch new file mode 100644 index 0000000..fd02da5 --- /dev/null +++ b/0008-Fix-coverity-warnings-on-dbus.patch @@ -0,0 +1,131 @@ +From bbfdf6a435cbd5f71ae76f962ce86786346589aa 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 17:19:05 +0200 +Subject: [PATCH 08/15] Fix coverity warnings on dbus + +Error: CLANG_WARNING: [#def30] +dnsmasq-2.86test7/src/dbus.c:117:3: warning[deadcode.DeadStores]: Value stored to 'w' is never read + # 115| daemon->watches = w; + # 116| + # 117|-> w = data; /* no warning */ + # 118| return TRUE; + # 119| } + +Error: CLANG_WARNING: [#def31] +dnsmasq-2.86test7/src/dbus.c:137:3: warning[deadcode.DeadStores]: Value stored to 'w' is never read + # 135| } + # 136| + # 137|-> w = data; /* no warning */ + # 138| } + # 139| + +Error: CHECKED_RETURN (CWE-252): [#def32] +dnsmasq-2.86test7/src/dbus.c:146: check_return: Calling "dbus_message_iter_init" without checking return value (as is done elsewhere 4 out of 5 times). +dnsmasq-2.86test7/src/dbus.c:460: example_checked: Example 1: "dbus_message_iter_init(message, &iter)" has its value checked in "dbus_message_iter_init(message, &iter)". +dnsmasq-2.86test7/src/dbus.c:573: example_checked: Example 2: "dbus_message_iter_init(message, &iter)" has its value checked in "dbus_message_iter_init(message, &iter)". +dnsmasq-2.86test7/src/dbus.c:257: example_checked: Example 3: "dbus_message_iter_init(message, &iter)" has its value checked in "dbus_message_iter_init(message, &iter)". +dnsmasq-2.86test7/src/dbus.c:427: example_checked: Example 4: "dbus_message_iter_init(message, &iter)" has its value checked in "dbus_message_iter_init(message, &iter)". + # 144| char *domain; + # 145| + # 146|-> dbus_message_iter_init(message, &iter); + # 147| + # 148| mark_servers(SERV_FROM_DBUS); + +Error: NEGATIVE_RETURNS (CWE-394): [#def33] +dnsmasq-2.86test7/src/dbus.c:547: negative_return_fn: Function "parse_hex((char *)hwaddr, dhcp_chaddr, 16, NULL, &hw_type)" returns a negative number. +dnsmasq-2.86test7/src/dbus.c:547: assign: Assigning: "hw_len" = "parse_hex((char *)hwaddr, dhcp_chaddr, 16, NULL, &hw_type)". +dnsmasq-2.86test7/src/dbus.c:551: negative_returns: "hw_len" is passed to a parameter that cannot be negative. + # 549| hw_type = ARPHRD_ETHER; + # 550| + # 551|-> lease_set_hwaddr(lease, dhcp_chaddr, clid, hw_len, hw_type, + # 552| clid_len, now, 0); + # 553| lease_set_expires(lease, expires, now); + +Error: CLANG_WARNING: [#def34] +dnsmasq-2.86test7/src/dbus.c:722:3: warning[deadcode.DeadStores]: Value stored to 'method' is never read + # 720| clear_cache_and_reload(dnsmasq_time()); + # 721| + # 722|-> method = user_data; /* no warning */ + # 723| + # 724| /* If no reply or no error, return nothing */ +--- + src/dbus.c | 21 +++++++++++++++------ + 1 file changed, 15 insertions(+), 6 deletions(-) + +diff --git a/src/dbus.c b/src/dbus.c +index cbdce9c..d746b9a 100644 +--- a/src/dbus.c ++++ b/src/dbus.c +@@ -114,7 +114,7 @@ static dbus_bool_t add_watch(DBusWatch *watch, void *data) + w->next = daemon->watches; + daemon->watches = w; + +- w = data; /* no warning */ ++ (void)data; /* no warning */ + return TRUE; + } + +@@ -134,16 +134,20 @@ static void remove_watch(DBusWatch *watch, void *data) + up = &(w->next); + } + +- w = data; /* no warning */ ++ (void)data; /* no warning */ + } + +-static void dbus_read_servers(DBusMessage *message) ++static DBusMessage* dbus_read_servers(DBusMessage *message) + { + DBusMessageIter iter; + union mysockaddr addr, source_addr; + char *domain; + +- dbus_message_iter_init(message, &iter); ++ if (!dbus_message_iter_init(message, &iter)) ++ { ++ return dbus_message_new_error(message, DBUS_ERROR_INVALID_ARGS, ++ "Failed to initialize dbus message iter"); ++ } + + mark_servers(SERV_FROM_DBUS); + +@@ -222,6 +226,7 @@ static void dbus_read_servers(DBusMessage *message) + + /* unlink and free anything still marked. */ + cleanup_servers(); ++ return NULL; + } + + #ifdef HAVE_LOOP +@@ -545,6 +550,10 @@ static DBusMessage *dbus_add_lease(DBusMessage* message) + "Invalid IP address '%s'", ipaddr); + + hw_len = parse_hex((char*)hwaddr, dhcp_chaddr, DHCP_CHADDR_MAX, NULL, &hw_type); ++ if (hw_len < 0) ++ return dbus_message_new_error_printf(message, DBUS_ERROR_INVALID_ARGS, ++ "Invalid HW address '%s'", hwaddr); ++ + if (hw_type == 0 && hw_len != 0) + hw_type = ARPHRD_ETHER; + +@@ -668,7 +677,7 @@ DBusHandlerResult message_handler(DBusConnection *connection, + #endif + else if (strcmp(method, "SetServers") == 0) + { +- dbus_read_servers(message); ++ reply = dbus_read_servers(message); + new_servers = 1; + } + else if (strcmp(method, "SetServersEx") == 0) +@@ -719,7 +728,7 @@ DBusHandlerResult message_handler(DBusConnection *connection, + if (clear_cache) + clear_cache_and_reload(dnsmasq_time()); + +- method = user_data; /* no warning */ ++ (void)user_data; /* no warning */ + + /* If no reply or no error, return nothing */ + if (!reply) +-- +2.31.1 + diff --git a/0009-Address-coverity-issues-detected-in-util.c.patch b/0009-Address-coverity-issues-detected-in-util.c.patch new file mode 100644 index 0000000..cc075c7 --- /dev/null +++ b/0009-Address-coverity-issues-detected-in-util.c.patch @@ -0,0 +1,110 @@ +From 7b975696a7bda5b86fcf168644f177544adb6fe9 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 17:38:26 +0200 +Subject: [PATCH 09/15] Address coverity issues detected in util.c + +Error: CLANG_WARNING: [#def163] +dnsmasq-2.86test7/src/util.c:204:9: warning[deadcode.DeadStores]: Although the value stored to 'rc' is used in the enclosing expression, the value is never actually read from 'rc' + # 202| *nomem = 0; + # 203| + # 204|-> if (!(rc = check_name(in))) + # 205| return NULL; + # 206| + +Error: UNREACHABLE (CWE-561): [#def164] +dnsmasq-2.86test7/src/util.c:239: unreachable: This code cannot be reached: "if (ret = whine_malloc(strl...". + # 237| #endif + # 238| + # 239|-> if ((ret = whine_malloc(strlen(in)+1))) + # 240| strcpy(ret, in); + # 241| else if (nomem) + +Error: CLANG_WARNING: [#def165] +dnsmasq-2.86test7/src/util.c:531:2: warning[deadcode.DeadStores]: Value stored to 'p' is never read + # 529| p += sprintf(&buf[p], "%um", x); + # 530| if ((x = t%60)) + # 531|-> p += sprintf(&buf[p], "%us", x); + # 532| } + # 533| } + +Error: CPPCHECK_WARNING (CWE-456): [#def166] +dnsmasq-2.86test7/src/util.c:577: error[uninitvar]: Uninitialized variable: sav + # 575| for (j = 0; j < bytes; j++) + # 576| { + # 577|-> char sav = sav; + # 578| if (j < bytes - 1) + # 579| { + +Error: CLANG_WARNING: [#def167] +dnsmasq-2.86test7/src/util.c:577:9: warning[core.uninitialized.Assign]: Assigned value is garbage or undefined + # 575| for (j = 0; j < bytes; j++) + # 576| { + # 577|-> char sav = sav; + # 578| if (j < bytes - 1) + # 579| { + +Error: MISSING_RESTORE (CWE-573): [#def168] +dnsmasq-2.86test7/src/util.c:580: save: Saving non-local "in[(j + 1) * 2]" in local "sav". +dnsmasq-2.86test7/src/util.c:581: modify: Modifying non-local "in[(j + 1) * 2]". +dnsmasq-2.86test7/src/util.c:586: end_of_scope: Value of non-local "in[(j + 1) * 2]" that was saved in "sav" is not restored as it was along other paths. +dnsmasq-2.86test7/src/util.c:592: restore_example: The original value of non-local "in[(j + 1) * 2]" was restored here. + # 584| is illegal. */ + # 585| if (strchr(&in[j*2], '*')) + # 586|-> return -1; + # 587| out[i] = strtol(&in[j*2], NULL, 16); + # 588| mask = mask << 1; +--- + src/util.c | 13 +++++++++---- + 1 file changed, 9 insertions(+), 4 deletions(-) + +diff --git a/src/util.c b/src/util.c +index 1425764..8e69d55 100644 +--- a/src/util.c ++++ b/src/util.c +@@ -208,6 +208,8 @@ char *canonicalise(char *in, int *nomem) + /* older libidn2 strips underscores, so don't do IDN processing + if the name has an underscore (check_name() returned 2) */ + if (rc != 2) ++#else ++ (void)rc; + #endif + #if defined(HAVE_IDN) || defined(HAVE_LIBIDN2) + { +@@ -235,11 +237,14 @@ char *canonicalise(char *in, int *nomem) + return ret; + } + #endif +- ++ ++#if !defined(HAVE_LIBIDN2) || (defined(HAVE_LIBIDN2) && (!defined(IDN2_VERSION_NUMBER) || IDN2_VERSION_NUMBER < 0x02000003)) ++ /* If recent libidn2 is used, it cannot reach this code. */ + if ((ret = whine_malloc(strlen(in)+1))) + strcpy(ret, in); + else if (nomem) +- *nomem = 1; ++ *nomem = 1; ++#endif + + return ret; + } +@@ -528,7 +533,7 @@ void prettyprint_time(char *buf, unsigned int t) + if ((x = (t/60)%60)) + p += sprintf(&buf[p], "%um", x); + if ((x = t%60)) +- p += sprintf(&buf[p], "%us", x); ++ sprintf(&buf[p], "%us", x); + } + } + +@@ -574,7 +579,7 @@ int parse_hex(char *in, unsigned char *out, int maxlen, + int j, bytes = (1 + (r - in))/2; + for (j = 0; j < bytes; j++) + { +- char sav = sav; ++ char sav; + if (j < bytes - 1) + { + sav = in[(j+1)*2]; +-- +2.31.1 + diff --git a/0010-Fix-coverity-detected-issues-in-option.c.patch b/0010-Fix-coverity-detected-issues-in-option.c.patch new file mode 100644 index 0000000..a63ac41 --- /dev/null +++ b/0010-Fix-coverity-detected-issues-in-option.c.patch @@ -0,0 +1,236 @@ +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 + diff --git a/0011-Fix-coverity-detected-issue-in-radv.c.patch b/0011-Fix-coverity-detected-issue-in-radv.c.patch new file mode 100644 index 0000000..845de43 --- /dev/null +++ b/0011-Fix-coverity-detected-issue-in-radv.c.patch @@ -0,0 +1,54 @@ +From 9c088b29dcdb8a3e013120d8272a6e0314a8f3df 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:29:23 +0200 +Subject: [PATCH 11/15] Fix coverity detected issue in radv.c + +Error: NULL_RETURNS (CWE-476): [#def114] +dnsmasq-2.86test7/src/radv.c:748: returned_null: "expand" returns "NULL" (checked 10 out of 11 times). +dnsmasq-2.86test7/src/radv.c:748: var_assigned: Assigning: "p" = "NULL" return value from "expand". +dnsmasq-2.86test7/src/radv.c:749: dereference: Dereferencing a pointer that might be "NULL" "p" when calling "memset". [Note: The source code implementation of the function has been overridden by a builtin model.] +dnsmasq-2.86test7/src/outpacket.c:83: example_checked: Example 1: "expand(len)" has its value checked in "p = expand(len)". +dnsmasq-2.86test7/src/outpacket.c:109: example_checked: Example 2: "expand(1UL)" has its value checked in "p = expand(1UL)". +dnsmasq-2.86test7/src/radv.c:269: example_checked: Example 3: "expand(16UL)" has its value checked in "ra = expand(16UL)". +dnsmasq-2.86test7/src/radv.c:363: example_checked: Example 4: "expand(32UL)" has its value checked in "opt = expand(32UL)". +dnsmasq-2.86test7/src/radv.c:708: example_checked: Example 5: "expand(32UL)" has its value checked in "opt = expand(32UL)". + # 747| int len = (maclen + 9) >> 3; + # 748| unsigned char *p = expand(len << 3); + # 749|-> memset(p, 0, len << 3); + # 750| *p++ = ICMP6_OPT_SOURCE_MAC; + # 751| *p++ = len; + +Error: NULL_RETURNS (CWE-476): [#def115] +dnsmasq-2.86test7/src/radv.c:748: returned_null: "expand" returns "NULL" (checked 10 out of 11 times). +dnsmasq-2.86test7/src/radv.c:748: var_assigned: Assigning: "p" = "NULL" return value from "expand". +dnsmasq-2.86test7/src/radv.c:750: dereference: Incrementing a pointer which might be null: "p". +dnsmasq-2.86test7/src/outpacket.c:83: example_checked: Example 1: "expand(len)" has its value checked in "p = expand(len)". +dnsmasq-2.86test7/src/outpacket.c:109: example_checked: Example 2: "expand(1UL)" has its value checked in "p = expand(1UL)". +dnsmasq-2.86test7/src/radv.c:269: example_checked: Example 3: "expand(16UL)" has its value checked in "ra = expand(16UL)". +dnsmasq-2.86test7/src/radv.c:363: example_checked: Example 4: "expand(32UL)" has its value checked in "opt = expand(32UL)". +dnsmasq-2.86test7/src/radv.c:708: example_checked: Example 5: "expand(32UL)" has its value checked in "opt = expand(32UL)". + # 748| unsigned char *p = expand(len << 3); + # 749| memset(p, 0, len << 3); + # 750|-> *p++ = ICMP6_OPT_SOURCE_MAC; + # 751| *p++ = len; + # 752| memcpy(p, mac, maclen); +--- + src/radv.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/radv.c b/src/radv.c +index 3255904..6d6fa32 100644 +--- a/src/radv.c ++++ b/src/radv.c +@@ -746,6 +746,8 @@ static int add_lla(int index, unsigned int type, char *mac, size_t maclen, void + add 7 to round up */ + int len = (maclen + 9) >> 3; + unsigned char *p = expand(len << 3); ++ if (!p) ++ return 1; + memset(p, 0, len << 3); + *p++ = ICMP6_OPT_SOURCE_MAC; + *p++ = len; +-- +2.31.1 + diff --git a/0012-Fix-coverity-detected-issues-in-cache.c.patch b/0012-Fix-coverity-detected-issues-in-cache.c.patch new file mode 100644 index 0000000..e88c64a --- /dev/null +++ b/0012-Fix-coverity-detected-issues-in-cache.c.patch @@ -0,0 +1,41 @@ +From 957b2b25238d82a6c3afced2ff0423ad171fb22e 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 20:10:37 +0200 +Subject: [PATCH 12/15] Fix coverity detected issues in cache.c + +Error: UNINIT (CWE-457): [#def27] +dnsmasq-2.86test7/src/cache.c:1193: var_decl: Declaring variable "lrec" without initializer. +dnsmasq-2.86test7/src/cache.c:1315: uninit_use_in_call: Using uninitialized value "lrec.ttd" when calling "make_non_terminals". + # 1313| { + # 1314| lrec.name.namep = txt->name; + # 1315|-> make_non_terminals(&lrec); + # 1316| } + # 1317| + +Error: CLANG_WARNING: [#def29] +dnsmasq-2.86test7/src/cache.c:1552:15: warning[core.uninitialized.Assign]: Assigned value is garbage or undefined + # 1550| { + # 1551| crecp->flags = (source->flags | F_NAMEP) & ~(F_IPV4 | F_IPV6 | F_CNAME | F_SRV | F_DNSKEY | F_DS | F_REVERSE); + # 1552|-> crecp->ttd = source->ttd; + # 1553| crecp->name.namep = name; + # 1554| +--- + src/cache.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/cache.c b/src/cache.c +index 97c51a7..6722fa6 100644 +--- a/src/cache.c ++++ b/src/cache.c +@@ -1188,7 +1188,7 @@ void cache_reload(void) + struct host_record *hr; + struct name_list *nl; + struct cname *a; +- struct crec lrec; ++ struct crec lrec = { 0, }; + struct mx_srv_record *mx; + struct txt_record *txt; + struct interface_name *intr; +-- +2.31.1 + diff --git a/0013-Fix-coverity-issues-detected-in-domain-match.c.patch b/0013-Fix-coverity-issues-detected-in-domain-match.c.patch new file mode 100644 index 0000000..60df62b --- /dev/null +++ b/0013-Fix-coverity-issues-detected-in-domain-match.c.patch @@ -0,0 +1,134 @@ +From 0dafe990a1395d597bc6022c3936769f7a0ddea7 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 21:16:22 +0200 +Subject: [PATCH 13/15] Fix coverity issues detected in domain-match.c + +Error: CHECKED_RETURN (CWE-252): [#def28] +dnsmasq-2.86rc3/src/domain-match.c:414: check_return: Calling "add_resource_record" without checking return value (as is done elsewhere 44 out of 46 times). +dnsmasq-2.86rc3/src/auth.c:214: example_checked: Example 1: "add_resource_record(header, limit, &trunc, nameoffset, &ansp, dnsmasq_daemon->auth_ttl, NULL, 12, 1, "d", intr->name)" has its value checked in "add_resource_record(header, limit, &trunc, nameoffset, &ansp, dnsmasq_daemon->auth_ttl, NULL, 12, 1, "d", intr->name)". +dnsmasq-2.86rc3/src/auth.c:239: example_checked: Example 2: "add_resource_record(header, limit, &trunc, nameoffset, &ansp, dnsmasq_daemon->auth_ttl, NULL, 12, 1, "d", name)" has its value checked in "add_resource_record(header, limit, &trunc, nameoffset, &ansp, dnsmasq_daemon->auth_ttl, NULL, 12, 1, "d", name)". +dnsmasq-2.86rc3/src/rfc1035.c:1463: example_checked: Example 3: "add_resource_record(header, limit, &trunc, nameoffset, &ansp, crec_ttl(crecp, now), &nameoffset, 5, 1, "d", cname_target)" has its value checked in "add_resource_record(header, limit, &trunc, nameoffset, &ansp, crec_ttl(crecp, now), &nameoffset, 5, 1, "d", cname_target)". +dnsmasq-2.86rc3/src/rfc1035.c:1500: example_checked: Example 4: "add_resource_record(header, limit, &trunc, nameoffset, &ansp, ttl, NULL, 16, t->class, "t", t->len, t->txt)" has its value checked in "add_resource_record(header, limit, &trunc, nameoffset, &ansp, ttl, NULL, 16, t->class, "t", t->len, t->txt)". +dnsmasq-2.86rc3/src/rfc1035.c:2021: example_checked: Example 5: "add_resource_record(header, limit, NULL, rec->offset, &ansp, crec_ttl(crecp, now), NULL, type, 1, ((crecp->flags & 0x80U) ? "4" : "6"), &crecp->addr)" has its value checked in "add_resource_record(header, limit, NULL, rec->offset, &ansp, crec_ttl(crecp, now), NULL, type, 1, ((crecp->flags & 0x80U) ? "4" : "6"), &crecp->addr)". + # 412| + # 413| header->ancount = htons(ntohs(header->ancount) + 1); + # 414|-> add_resource_record(header, limit, &trunc, sizeof(struct dns_header), &p, daemon->local_ttl, NULL, T_A, C_IN, "4", &addr); + # 415| log_query((flags | F_CONFIG | F_FORWARD) & ~F_IPV6, name, (union all_addr *)&addr, NULL); + # 416| } + +Error: CHECKED_RETURN (CWE-252): [#def29] +dnsmasq-2.86rc3/src/domain-match.c:429: check_return: Calling "add_resource_record" without checking return value (as is done elsewhere 44 out of 46 times). +dnsmasq-2.86rc3/src/auth.c:214: example_checked: Example 1: "add_resource_record(header, limit, &trunc, nameoffset, &ansp, dnsmasq_daemon->auth_ttl, NULL, 12, 1, "d", intr->name)" has its value checked in "add_resource_record(header, limit, &trunc, nameoffset, &ansp, dnsmasq_daemon->auth_ttl, NULL, 12, 1, "d", intr->name)". +dnsmasq-2.86rc3/src/auth.c:239: example_checked: Example 2: "add_resource_record(header, limit, &trunc, nameoffset, &ansp, dnsmasq_daemon->auth_ttl, NULL, 12, 1, "d", name)" has its value checked in "add_resource_record(header, limit, &trunc, nameoffset, &ansp, dnsmasq_daemon->auth_ttl, NULL, 12, 1, "d", name)". +dnsmasq-2.86rc3/src/rfc1035.c:1463: example_checked: Example 3: "add_resource_record(header, limit, &trunc, nameoffset, &ansp, crec_ttl(crecp, now), &nameoffset, 5, 1, "d", cname_target)" has its value checked in "add_resource_record(header, limit, &trunc, nameoffset, &ansp, crec_ttl(crecp, now), &nameoffset, 5, 1, "d", cname_target)". +dnsmasq-2.86rc3/src/rfc1035.c:1500: example_checked: Example 4: "add_resource_record(header, limit, &trunc, nameoffset, &ansp, ttl, NULL, 16, t->class, "t", t->len, t->txt)" has its value checked in "add_resource_record(header, limit, &trunc, nameoffset, &ansp, ttl, NULL, 16, t->class, "t", t->len, t->txt)". +dnsmasq-2.86rc3/src/rfc1035.c:2021: example_checked: Example 5: "add_resource_record(header, limit, NULL, rec->offset, &ansp, crec_ttl(crecp, now), NULL, type, 1, ((crecp->flags & 0x80U) ? "4" : "6"), &crecp->addr)" has its value checked in "add_resource_record(header, limit, NULL, rec->offset, &ansp, crec_ttl(crecp, now), NULL, type, 1, ((crecp->flags & 0x80U) ? "4" : "6"), &crecp->addr)". + # 427| + # 428| header->ancount = htons(ntohs(header->ancount) + 1); + # 429|-> add_resource_record(header, limit, &trunc, sizeof(struct dns_header), &p, daemon->local_ttl, NULL, T_AAAA, C_IN, "6", &addr); + # 430| log_query((flags | F_CONFIG | F_FORWARD) & ~F_IPV4, name, (union all_addr *)&addr, NULL); + # 431| } + +Error: NULL_RETURNS (CWE-476): [#def30] +dnsmasq-2.86rc3/src/domain-match.c:611: returned_null: "whine_malloc" returns "NULL" (checked 72 out of 76 times). +dnsmasq-2.86rc3/src/domain-match.c:611: var_assigned: Assigning: "alloc_domain" = "NULL" return value from "whine_malloc". +dnsmasq-2.86rc3/src/domain-match.c:620: dereference: Dereferencing a pointer that might be "NULL" "alloc_domain" when calling "hostname_isequal". +dnsmasq-2.86rc3/src/arp.c:88: example_checked: Example 1: "whine_malloc(48UL)" has its value checked in "arp = whine_malloc(48UL)". +dnsmasq-2.86rc3/src/blockdata.c:24: example_assign: Example 2: Assigning: "new" = return value from "whine_malloc(n * 48UL)". +dnsmasq-2.86rc3/src/blockdata.c:26: example_checked: Example 2 (cont.): "new" has its value checked in "new". +dnsmasq-2.86rc3/src/cache.c:1545: example_assign: Example 3: Assigning: "crecp" = return value from "whine_malloc(70UL)". +dnsmasq-2.86rc3/src/cache.c:1547: example_checked: Example 3 (cont.): "crecp" has its value checked in "crecp". +dnsmasq-2.86rc3/src/forward.c:1791: example_assign: Example 4: Assigning: "packet" = return value from "whine_malloc(66573UL)". +dnsmasq-2.86rc3/src/forward.c:1795: example_checked: Example 4 (cont.): "packet" has its value checked in "packet". +dnsmasq-2.86rc3/src/inotify.c:186: example_checked: Example 5: "whine_malloc(lendir + lenfile + 2UL)" has its value checked in "path = whine_malloc(lendir + lenfile + 2UL)". + # 618| if (flags & SERV_IS_LOCAL) + # 619| for (serv = daemon->servers; serv; serv = serv->next) + # 620|-> if ((serv->flags & SERV_MARK) && + # 621| hostname_isequal(alloc_domain, serv->domain)) + # 622| break; + +Error: RESOURCE_LEAK (CWE-772): [#def31] [important] +dnsmasq-2.86rc3/src/domain-match.c:611: alloc_fn: Storage is returned from allocation function "whine_malloc". +dnsmasq-2.86rc3/src/domain-match.c:611: var_assign: Assigning: "alloc_domain" = storage returned from "whine_malloc(1UL)". +dnsmasq-2.86rc3/src/domain-match.c:620: noescape: Resource "alloc_domain" is not freed or pointed-to in "hostname_isequal". +dnsmasq-2.86rc3/src/domain-match.c:646: leaked_storage: Variable "alloc_domain" going out of scope leaks the storage it points to. + # 644| + # 645| if (!(serv = whine_malloc(size))) + # 646|-> return 0; + # 647| + # 648| if (flags & SERV_IS_LOCAL) + +Error: NULL_RETURNS (CWE-476): [#def32] +dnsmasq-2.86rc3/src/domain-match.c:611: returned_null: "whine_malloc" returns "NULL" (checked 72 out of 76 times). +dnsmasq-2.86rc3/src/domain-match.c:611: var_assigned: Assigning: "alloc_domain" = "NULL" return value from "whine_malloc". +dnsmasq-2.86rc3/src/domain-match.c:674: dereference: Dereferencing a pointer that might be "NULL" "alloc_domain" when calling "strlen". +dnsmasq-2.86rc3/src/arp.c:88: example_checked: Example 1: "whine_malloc(48UL)" has its value checked in "arp = whine_malloc(48UL)". +dnsmasq-2.86rc3/src/blockdata.c:24: example_assign: Example 2: Assigning: "new" = return value from "whine_malloc(n * 48UL)". +dnsmasq-2.86rc3/src/blockdata.c:26: example_checked: Example 2 (cont.): "new" has its value checked in "new". +dnsmasq-2.86rc3/src/cache.c:1545: example_assign: Example 3: Assigning: "crecp" = return value from "whine_malloc(70UL)". +dnsmasq-2.86rc3/src/cache.c:1547: example_checked: Example 3 (cont.): "crecp" has its value checked in "crecp". +dnsmasq-2.86rc3/src/forward.c:1791: example_assign: Example 4: Assigning: "packet" = return value from "whine_malloc(66573UL)". +dnsmasq-2.86rc3/src/forward.c:1795: example_checked: Example 4 (cont.): "packet" has its value checked in "packet". +dnsmasq-2.86rc3/src/inotify.c:186: example_checked: Example 5: "whine_malloc(lendir + lenfile + 2UL)" has its value checked in "path = whine_malloc(lendir + lenfile + 2UL)". + # 672| serv->flags = flags; + # 673| serv->domain = alloc_domain; + # 674|-> serv->domain_len = strlen(alloc_domain); + # 675| + # 676| if (flags & SERV_4ADDR) +--- + src/domain-match.c | 17 ++++++++++++----- + 1 file changed, 12 insertions(+), 5 deletions(-) + +diff --git a/src/domain-match.c b/src/domain-match.c +index f8e4796..7124c18 100644 +--- a/src/domain-match.c ++++ b/src/domain-match.c +@@ -411,7 +411,8 @@ size_t make_local_answer(int flags, int gotname, size_t size, struct dns_header + addr.addr4 = srv->addr; + + header->ancount = htons(ntohs(header->ancount) + 1); +- add_resource_record(header, limit, &trunc, sizeof(struct dns_header), &p, daemon->local_ttl, NULL, T_A, C_IN, "4", &addr); ++ if (!add_resource_record(header, limit, &trunc, sizeof(struct dns_header), &p, daemon->local_ttl, NULL, T_A, C_IN, "4", &addr)) ++ return 0; + log_query((flags | F_CONFIG | F_FORWARD) & ~F_IPV6, name, (union all_addr *)&addr, NULL); + } + +@@ -426,7 +427,8 @@ size_t make_local_answer(int flags, int gotname, size_t size, struct dns_header + addr.addr6 = srv->addr; + + header->ancount = htons(ntohs(header->ancount) + 1); +- add_resource_record(header, limit, &trunc, sizeof(struct dns_header), &p, daemon->local_ttl, NULL, T_AAAA, C_IN, "6", &addr); ++ if (!add_resource_record(header, limit, &trunc, sizeof(struct dns_header), &p, daemon->local_ttl, NULL, T_AAAA, C_IN, "6", &addr)) ++ return 0; + log_query((flags | F_CONFIG | F_FORWARD) & ~F_IPV4, name, (union all_addr *)&addr, NULL); + } + +@@ -609,9 +611,11 @@ int add_update_server(int flags, + + if (*domain == 0) + alloc_domain = whine_malloc(1); +- else if (!(alloc_domain = canonicalise((char *)domain, NULL))) ++ else ++ alloc_domain = canonicalise((char *)domain, NULL); ++ if (!alloc_domain) + return 0; +- ++ + /* See if there is a suitable candidate, and unmark + only do this for forwarding servers, not + address or local, to avoid delays on large numbers. */ +@@ -643,7 +647,10 @@ int add_update_server(int flags, + size = sizeof(struct server); + + if (!(serv = whine_malloc(size))) +- return 0; ++ { ++ free(alloc_domain); ++ return 0; ++ } + + if (flags & SERV_IS_LOCAL) + { +-- +2.31.1 + diff --git a/0014-Fix-coverity-detected-issues-in-dnsmasq.c.patch b/0014-Fix-coverity-detected-issues-in-dnsmasq.c.patch new file mode 100644 index 0000000..6069244 --- /dev/null +++ b/0014-Fix-coverity-detected-issues-in-dnsmasq.c.patch @@ -0,0 +1,133 @@ +From f476acbe3c2830e6ff0c50cc36d364a3f3f4fadb 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 22:45:29 +0200 +Subject: [PATCH 14/15] Fix coverity detected issues in dnsmasq.c + +Error: DEADCODE (CWE-561): [#def12] +dnsmasq-2.86rc3/src/dnsmasq.c:37: assignment: Assigning: "bind_fallback" = "0". +dnsmasq-2.86rc3/src/dnsmasq.c:927: const: At condition "bind_fallback", the value of "bind_fallback" must be equal to 0. +dnsmasq-2.86rc3/src/dnsmasq.c:927: dead_error_condition: The condition "bind_fallback" cannot be true. +dnsmasq-2.86rc3/src/dnsmasq.c:928: dead_error_line: Execution cannot reach this statement: "my_syslog(4, "setting --bin...". +dnsmasq-2.86rc3/src/dnsmasq.c:928: effectively_constant: Local variable "bind_fallback" is assigned only once, to a constant value, making it effectively constant throughout its scope. If this is not the intent, examine the logic to see if there is a missing assignment that would make "bind_fallback" not remain constant. + # 926| + # 927| if (bind_fallback) + # 928|-> my_syslog(LOG_WARNING, _("setting --bind-interfaces option because of OS limitations")); + # 929| + # 930| if (option_bool(OPT_NOWILD)) + +Error: REVERSE_NEGATIVE (CWE-191): [#def13] +dnsmasq-2.86rc3/src/dnsmasq.c:383: negative_sink_in_call: Passing "dnsmasq_daemon->pxefd" to a parameter that cannot be negative. +dnsmasq-2.86rc3/src/dnsmasq.c:1086: check_after_sink: You might be using variable "dnsmasq_daemon->pxefd" before verifying that it is >= 0. + # 1084| { + # 1085| poll_listen(daemon->dhcpfd, POLLIN); + # 1086|-> if (daemon->pxefd != -1) + # 1087| poll_listen(daemon->pxefd, POLLIN); + # 1088| } + +Error: CHECKED_RETURN (CWE-252): [#def18] +dnsmasq-2.86rc3/src/dnsmasq.c:1582: check_return: Calling "fcntl(dnsmasq_daemon->helperfd, 4, i & 0xfffffffffffff7ff)" without checking return value. This library function may fail and return an error code. + # 1580| /* block in writes until all done */ + # 1581| if ((i = fcntl(daemon->helperfd, F_GETFL)) != -1) + # 1582|-> fcntl(daemon->helperfd, F_SETFL, i & ~O_NONBLOCK); + # 1583| do { + # 1584| helper_write(); + +Error: CHECKED_RETURN (CWE-252): [#def22] +dnsmasq-2.86rc3/src/dnsmasq.c:1991: check_return: Calling "fcntl(confd, 4, flags & 0xfffffffffffff7ff)" without checking return value. This library function may fail and return an error code. + # 1989| Reset that here. */ + # 1990| if ((flags = fcntl(confd, F_GETFL, 0)) != -1) + # 1991|-> fcntl(confd, F_SETFL, flags & ~O_NONBLOCK); + # 1992| + # 1993| buff = tcp_request(confd, now, &tcp_addr, netmask, auth_dns); + +Error: CHECKED_RETURN (CWE-252): [#def26] +dnsmasq-2.86rc3/src/dnssec.c:727: check_return: Calling "extract_name" without checking return value (as is done elsewhere 9 out of 10 times). +dnsmasq-2.86rc3/src/dnssec.c:459: example_checked: Example 1: "extract_name(header, plen, &p, keyname, 1, 0)" has its value checked in "extract_name(header, plen, &p, keyname, 1, 0)". +dnsmasq-2.86rc3/src/dnssec.c:269: example_checked: Example 2: "extract_name(header, plen, &state->ip, state->buff, 1, 0)" has its value checked in "extract_name(header, plen, &state->ip, state->buff, 1, 0)". +dnsmasq-2.86rc3/src/dnssec.c:569: example_checked: Example 3: "extract_name(header, plen, &p, keyname, 1, 0)" has its value checked in "extract_name(header, plen, &p, keyname, 1, 0)". +dnsmasq-2.86rc3/src/rfc1035.c:648: example_checked: Example 4: "extract_name(header, qlen, &p1, name, 1, 0)" has its value checked in "extract_name(header, qlen, &p1, name, 1, 0)". +dnsmasq-2.86rc3/src/rfc1035.c:787: example_checked: Example 5: "extract_name(header, qlen, &p1, name, 1, 0)" has its value checked in "extract_name(header, qlen, &p1, name, 1, 0)". + # 725| /* namebuff used for workspace above, restore to leave unchanged on exit */ + # 726| p = (unsigned char*)(rrset[0]); + # 727|-> extract_name(header, plen, &p, name, 1, 0); + # 728| + # 729| if (key) + +Error: CHECKED_RETURN (CWE-252): [#def27] +dnsmasq-2.86rc3/src/dnssec.c:1020: check_return: Calling "extract_name" without checking return value (as is done elsewhere 7 out of 8 times). +dnsmasq-2.86rc3/src/auth.c:140: example_checked: Example 1: "extract_name(header, qlen, &p, name, 1, 4)" has its value checked in "extract_name(header, qlen, &p, name, 1, 4)". +dnsmasq-2.86rc3/src/dnssec.c:771: example_checked: Example 2: "extract_name(header, plen, &p, name, 1, 4)" has its value checked in "extract_name(header, plen, &p, name, 1, 4)". +dnsmasq-2.86rc3/src/hash-questions.c:57: example_checked: Example 3: "extract_name(header, plen, &p, name, 1, 4)" has its value checked in "extract_name(header, plen, &p, name, 1, 4)". +dnsmasq-2.86rc3/src/rfc1035.c:1028: example_checked: Example 4: "extract_name(header, qlen, &p, name, 1, 4)" has its value checked in "extract_name(header, qlen, &p, name, 1, 4)". +dnsmasq-2.86rc3/src/rfc1035.c:1438: example_checked: Example 5: "extract_name(header, qlen, &p, name, 1, 4)" has its value checked in "extract_name(header, qlen, &p, name, 1, 4)". + # 1018| + # 1019| p = (unsigned char *)(header+1); + # 1020|-> extract_name(header, plen, &p, name, 1, 4); + # 1021| p += 4; /* qtype, qclass */ + # 1022| +--- + src/dnsmasq.c | 11 +++++++---- + 1 file changed, 7 insertions(+), 4 deletions(-) + +diff --git a/src/dnsmasq.c b/src/dnsmasq.c +index 602daed..3e1bfe8 100644 +--- a/src/dnsmasq.c ++++ b/src/dnsmasq.c +@@ -34,7 +34,6 @@ static void poll_resolv(int force, int do_reload, time_t now); + + int main (int argc, char **argv) + { +- int bind_fallback = 0; + time_t now; + struct sigaction sigact; + struct iname *if_tmp; +@@ -59,6 +58,8 @@ int main (int argc, char **argv) + int did_bind = 0; + struct server *serv; + char *netlink_warn; ++#else ++ int bind_fallback = 0; + #endif + #if defined(HAVE_DHCP) || defined(HAVE_DHCP6) + struct dhcp_context *context; +@@ -377,7 +378,7 @@ int main (int argc, char **argv) + bindtodevice(bound_device, daemon->dhcpfd); + did_bind = 1; + } +- if (daemon->enable_pxe && bound_device) ++ if (daemon->enable_pxe && bound_device && daemon->pxefd != -1) + { + bindtodevice(bound_device, daemon->pxefd); + did_bind = 1; +@@ -920,8 +921,10 @@ int main (int argc, char **argv) + my_syslog(LOG_WARNING, _("warning: failed to change owner of %s: %s"), + daemon->log_file, strerror(log_err)); + ++#ifndef HAVE_LINUX_NETWORK + if (bind_fallback) + my_syslog(LOG_WARNING, _("setting --bind-interfaces option because of OS limitations")); ++#endif + + if (option_bool(OPT_NOWILD)) + warn_bound_listeners(); +@@ -1575,7 +1578,7 @@ static void async_event(int pipe, time_t now) + { + /* block in writes until all done */ + if ((i = fcntl(daemon->helperfd, F_GETFL)) != -1) +- fcntl(daemon->helperfd, F_SETFL, i & ~O_NONBLOCK); ++ while(retry_send(fcntl(daemon->helperfd, F_SETFL, i & ~O_NONBLOCK))); + do { + helper_write(); + } while (!helper_buf_empty() || do_script_run(now)); +@@ -1984,7 +1987,7 @@ static void check_dns_listeners(time_t now) + attribute from the listening socket. + Reset that here. */ + if ((flags = fcntl(confd, F_GETFL, 0)) != -1) +- fcntl(confd, F_SETFL, flags & ~O_NONBLOCK); ++ while(retry_send(fcntl(confd, F_SETFL, flags & ~O_NONBLOCK))); + + buff = tcp_request(confd, now, &tcp_addr, netmask, auth_dns); + +-- +2.31.1 + diff --git a/0015-Fix-coverity-issues-in-dnssec.c.patch b/0015-Fix-coverity-issues-in-dnssec.c.patch new file mode 100644 index 0000000..67b1d6d --- /dev/null +++ b/0015-Fix-coverity-issues-in-dnssec.c.patch @@ -0,0 +1,62 @@ +From 82c23fb1f0d9e46c6ce4bc4a57f0d377cc6089b7 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 22:51:36 +0200 +Subject: [PATCH 15/15] Fix coverity issues in dnssec.c + +Error: CHECKED_RETURN (CWE-252): [#def26] +dnsmasq-2.86rc3/src/dnssec.c:727: check_return: Calling "extract_name" without checking return value (as is done elsewhere 9 out of 10 times). +dnsmasq-2.86rc3/src/dnssec.c:459: example_checked: Example 1: "extract_name(header, plen, &p, keyname, 1, 0)" has its value checked in "extract_name(header, plen, &p, keyname, 1, 0)". +dnsmasq-2.86rc3/src/dnssec.c:269: example_checked: Example 2: "extract_name(header, plen, &state->ip, state->buff, 1, 0)" has its value checked in "extract_name(header, plen, &state->ip, state->buff, 1, 0)". +dnsmasq-2.86rc3/src/dnssec.c:569: example_checked: Example 3: "extract_name(header, plen, &p, keyname, 1, 0)" has its value checked in "extract_name(header, plen, &p, keyname, 1, 0)". +dnsmasq-2.86rc3/src/rfc1035.c:648: example_checked: Example 4: "extract_name(header, qlen, &p1, name, 1, 0)" has its value checked in "extract_name(header, qlen, &p1, name, 1, 0)". +dnsmasq-2.86rc3/src/rfc1035.c:787: example_checked: Example 5: "extract_name(header, qlen, &p1, name, 1, 0)" has its value checked in "extract_name(header, qlen, &p1, name, 1, 0)". + # 725| /* namebuff used for workspace above, restore to leave unchanged on exit */ + # 726| p = (unsigned char*)(rrset[0]); + # 727|-> extract_name(header, plen, &p, name, 1, 0); + # 728| + # 729| if (key) + +Error: CHECKED_RETURN (CWE-252): [#def27] +dnsmasq-2.86rc3/src/dnssec.c:1020: check_return: Calling "extract_name" without checking return value (as is done elsewhere 7 out of 8 times). +dnsmasq-2.86rc3/src/auth.c:140: example_checked: Example 1: "extract_name(header, qlen, &p, name, 1, 4)" has its value checked in "extract_name(header, qlen, &p, name, 1, 4)". +dnsmasq-2.86rc3/src/dnssec.c:771: example_checked: Example 2: "extract_name(header, plen, &p, name, 1, 4)" has its value checked in "extract_name(header, plen, &p, name, 1, 4)". +dnsmasq-2.86rc3/src/hash-questions.c:57: example_checked: Example 3: "extract_name(header, plen, &p, name, 1, 4)" has its value checked in "extract_name(header, plen, &p, name, 1, 4)". +dnsmasq-2.86rc3/src/rfc1035.c:1028: example_checked: Example 4: "extract_name(header, qlen, &p, name, 1, 4)" has its value checked in "extract_name(header, qlen, &p, name, 1, 4)". +dnsmasq-2.86rc3/src/rfc1035.c:1438: example_checked: Example 5: "extract_name(header, qlen, &p, name, 1, 4)" has its value checked in "extract_name(header, qlen, &p, name, 1, 4)". + # 1018| + # 1019| p = (unsigned char *)(header+1); + # 1020|-> extract_name(header, plen, &p, name, 1, 4); + # 1021| p += 4; /* qtype, qclass */ + # 1022| +--- + src/dnssec.c | 7 +++++-- + 1 file changed, 5 insertions(+), 2 deletions(-) + +diff --git a/src/dnssec.c b/src/dnssec.c +index 94ebb6f..8800a5b 100644 +--- a/src/dnssec.c ++++ b/src/dnssec.c +@@ -724,7 +724,8 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in + + /* namebuff used for workspace above, restore to leave unchanged on exit */ + p = (unsigned char*)(rrset[0]); +- extract_name(header, plen, &p, name, 1, 0); ++ if (!extract_name(header, plen, &p, name, 1, 0)) ++ return STAT_BOGUS; + + if (key) + { +@@ -1017,7 +1018,9 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char + } + + p = (unsigned char *)(header+1); +- extract_name(header, plen, &p, name, 1, 4); ++ if (!extract_name(header, plen, &p, name, 1, 4)) ++ return STAT_BOGUS; ++ + p += 4; /* qtype, qclass */ + + /* If the key needed to validate the DS is on the same domain as the DS, we'll +-- +2.31.1 + diff --git a/dnsmasq.spec b/dnsmasq.spec index 7e74130..3af1202 100644 --- a/dnsmasq.spec +++ b/dnsmasq.spec @@ -41,8 +41,24 @@ Patch1: dnsmasq-2.77-underflow.patch # https://bugzilla.redhat.com/show_bug.cgi?id=1852373 Patch2: dnsmasq-2.81-configuration.patch Patch3: dnsmasq-2.78-fips.patch -# https://bugzilla.redhat.com/show_bug.cgi?id=1978718 -Patch4: dnsmasq-2.85-lease-hostname.patch + +Patch10: 0001-Retry-on-interrupted-error-in-tftp.patch +Patch11: 0002-Add-safety-checks-to-places-pointed-by-Coverity.patch +Patch12: 0003-Small-safeguard-to-unexpected-data.patch +Patch13: 0004-Fix-bunch-of-warnings-in-auth.c.patch +Patch14: 0005-Fix-few-coverity-warnings-in-lease-tools.patch +Patch15: 0006-Fix-coverity-formats-issues-in-blockdata.patch +Patch16: 0007-Retry-dhcp6-ping-on-interrupts.patch +Patch17: 0008-Fix-coverity-warnings-on-dbus.patch +Patch18: 0009-Address-coverity-issues-detected-in-util.c.patch +Patch19: 0010-Fix-coverity-detected-issues-in-option.c.patch +Patch20: 0011-Fix-coverity-detected-issue-in-radv.c.patch +Patch21: 0012-Fix-coverity-detected-issues-in-cache.c.patch +Patch22: 0013-Fix-coverity-issues-detected-in-domain-match.c.patch +Patch23: 0014-Fix-coverity-detected-issues-in-dnsmasq.c.patch +Patch24: 0015-Fix-coverity-issues-in-dnssec.c.patch + +
Requires: nettle
https://src.fedoraproject.org/rpms/dnsmasq/c/06b5c95b058e4985a844a72bfc98ab0...
scm-commits@lists.fedoraproject.org
-
notifications＠fedoraproject.org