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