res_resolver_unbound: Test for NULL ub_result in unbound_resolver_callback

The ub_result pointer passed to unbound_resolver_callback by
libunbound can be NULL if the query was for something malformed
like `.1` or `[.1]`.  If it is, we now set a 'ns_r_formerr' result
and return instead of crashing with a SEGV.  This causes pjproject
to simply cancel the transaction with a "No answer record in the DNS
response" error.  The existing "off nominal" unit test was also
updated to check this condition.

Although not necessary for this fix, we also made
ast_dns_resolver_completed() tolerant of a NULL result.

Resolves: GHSA-v428-g3cw-7hv9

diff --git a/main/dns_core.c b/main/dns_core.c
index b2b9d1b06629dcab16d77e893bb6f0a7f9f3f31b..7f3f2b6556f2e587d140dbff7fe00327fb57abc9 100644 (file)
--- a/main/dns_core.c
+++ b/main/dns_core.c
@@ -598,7 +598,9 @@ static void sort_result(int rr_type, struct ast_dns_result *result)
 
 void ast_dns_resolver_completed(struct ast_dns_query *query)
 {
-       sort_result(ast_dns_query_get_rr_type(query), query->result);
+       if (query->result) {
+               sort_result(ast_dns_query_get_rr_type(query), query->result);
+       }
 
        query->callback(query);
 }

diff --git a/res/res_resolver_unbound.c b/res/res_resolver_unbound.c
index 93bb3cadafd8d5f37ee0e6a96a8b5ed6adf47155..d0564b6ad2365413d3014d5fe62a4c850cf767d0 100644 (file)
--- a/res/res_resolver_unbound.c
+++ b/res/res_resolver_unbound.c
@@ -257,6 +257,14 @@ static void unbound_resolver_callback(void *data, int err, struct ub_result *ub_
 {
        struct ast_dns_query *query = data;
 
+       if (!ub_result) {
+               ast_debug(3, "Badly formatted DNS query '%s'\n", ast_dns_query_get_name(query));
+               ast_dns_resolver_set_result(query, 0, 0, ns_r_formerr, ast_dns_query_get_name(query), "", 0);
+               ast_dns_resolver_completed(query);
+               ao2_ref(query, -1);
+               return;
+       }
+
        if (!ast_dns_resolver_set_result(query, ub_result->secure, ub_result->bogus, ub_result->rcode,
                S_OR(ub_result->canonname, ast_dns_query_get_name(query)), ub_result->answer_packet, ub_result->answer_len)) {
                int i;
@@ -893,7 +901,8 @@ static int off_nominal_sync_run(struct ast_test *test, const char *domain, int r
        }
 
        if (ast_dns_result_get_rcode(result) != expected_rcode) {
-               ast_test_status_update(test, "Unexpected rcode from DNS resolution\n");
+               ast_test_status_update(test, "Unexpected rcode '%d' (expected '%d') from DNS resolution of '%s' class: '%d' type: '%d'\n",
+                       ast_dns_result_get_rcode(result), expected_rcode, domain, rr_class, rr_type);
                res = -1;
        }
 
@@ -1022,6 +1031,8 @@ static enum ast_test_result_state off_nominal_test(struct ast_test *test,
 
        static const char *DOMAIN1 = "goose.feathers";
        static const char *DOMAIN2 = "duck.feathers";
+       static const char *BADFORMAT1 = ".1";
+       static const char *BADFORMAT2 = ".www";
 
        static const char *ADDR1 = "127.0.0.2";
 
@@ -1043,6 +1054,10 @@ static enum ast_test_result_state off_nominal_test(struct ast_test *test,
                { DOMAIN2, ns_t_a,    ns_c_in, ns_r_nxdomain },
                { DOMAIN1, ns_t_aaaa, ns_c_in, ns_r_noerror },
                { DOMAIN1, ns_t_a,    ns_c_chaos, ns_r_refused },
+               { BADFORMAT1, ns_t_a,   ns_c_in, ns_r_formerr },
+               { BADFORMAT2, ns_t_a,   ns_c_in, ns_r_formerr },
+               { BADFORMAT1, ns_t_ptr, ns_c_in, ns_r_formerr },
+               { BADFORMAT2, ns_t_ptr, ns_c_in, ns_r_formerr },
        };
 
        inet_pton(AF_INET,  ADDR1, addr1_buf);