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