From b506faaa2d6674a11e351842ba1d1cd846a6c244 Mon Sep 17 00:00:00 2001 From: George Joseph Date: Mon, 12 Aug 2024 11:58:12 -0600 Subject: [PATCH] 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 (cherry picked from commit d2d0c507ffd68f40e3eb0875854a54a7a233188e) --- main/dns_core.c | 4 +++- res/res_resolver_unbound.c | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/main/dns_core.c b/main/dns_core.c index b2b9d1b066..7f3f2b6556 100644 --- 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 93bb3cadaf..d0564b6ad2 100644 --- 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);