From f68992a159f2ef02a482963116cd6b62d96f858c Mon Sep 17 00:00:00 2001 From: Brad House Date: Thu, 18 Jul 2024 10:55:24 -0400 Subject: [PATCH] Propagate record duplication error code (#820) In c-ares 1.30.0 we started validating strings parsed are printable. This caused a regression in a pycares test case due to a wrong response code being returned as the error was being propagated from a different section of code that was assuming the only possible failure condition was out-of-memory. This PR adds a fix for this and also a test case to validate it. Ref: https://github.com/saghul/pycares/issues/200 Fix By: Brad House (@bradh352) --- src/lib/ares_dns_private.h | 2 ++ src/lib/ares_dns_record.c | 31 ++++++++++++++++++++----------- src/lib/ares_send.c | 8 ++++---- test/ares-test-mock.cc | 15 +++++++++++++++ 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/lib/ares_dns_private.h b/src/lib/ares_dns_private.h index c82fb6a9..2dd468a5 100644 --- a/src/lib/ares_dns_private.h +++ b/src/lib/ares_dns_private.h @@ -26,6 +26,8 @@ #ifndef __ARES_DNS_PRIVATE_H #define __ARES_DNS_PRIVATE_H +ares_status_t ares_dns_record_duplicate_ex(ares_dns_record_t **dest, + const ares_dns_record_t *src); ares_bool_t ares_dns_rec_type_allow_name_compression(ares_dns_rec_type_t type); ares_bool_t ares_dns_opcode_isvalid(ares_dns_opcode_t opcode); ares_bool_t ares_dns_rcode_isvalid(ares_dns_rcode_t rcode); diff --git a/src/lib/ares_dns_record.c b/src/lib/ares_dns_record.c index 0131743e..8545abec 100644 --- a/src/lib/ares_dns_record.c +++ b/src/lib/ares_dns_record.c @@ -296,6 +296,7 @@ ares_status_t ares_dns_record_query_set_name(ares_dns_record_t *dnsrec, if (dnsrec == NULL || idx >= dnsrec->qdcount || name == NULL) { return ARES_EFORMERR; } + orig_name = dnsrec->qd[idx].name; dnsrec->qd[idx].name = ares_strdup(name); if (dnsrec->qd[idx].name == NULL) { @@ -1622,26 +1623,34 @@ done: return status; } -ares_dns_record_t *ares_dns_record_duplicate(const ares_dns_record_t *dnsrec) +ares_status_t ares_dns_record_duplicate_ex(ares_dns_record_t **dest, + const ares_dns_record_t *src) { unsigned char *data = NULL; size_t data_len = 0; - ares_dns_record_t *out = NULL; ares_status_t status; - if (dnsrec == NULL) { - return NULL; + if (dest == NULL || src == NULL) { + return ARES_EFORMERR; } - status = ares_dns_write(dnsrec, &data, &data_len); + *dest = NULL; + + status = ares_dns_write(src, &data, &data_len); if (status != ARES_SUCCESS) { - return NULL; + return status; } - status = ares_dns_parse(data, data_len, 0, &out); + status = ares_dns_parse(data, data_len, 0, dest); ares_free(data); - if (status != ARES_SUCCESS) { - return NULL; - } - return out; + + return status; +} + +ares_dns_record_t *ares_dns_record_duplicate(const ares_dns_record_t *dnsrec) +{ + ares_dns_record_t *dest = NULL; + + ares_dns_record_duplicate_ex(&dest, dnsrec); + return dest; } diff --git a/src/lib/ares_send.c b/src/lib/ares_send.c index f6614c62..9839ab31 100644 --- a/src/lib/ares_send.c +++ b/src/lib/ares_send.c @@ -145,11 +145,11 @@ ares_status_t ares_send_nolock(ares_channel_t *channel, query->using_tcp = (channel->flags & ARES_FLAG_USEVC)?ARES_TRUE:ARES_FALSE; /* Duplicate Query */ - query->query = ares_dns_record_duplicate(dnsrec); - if (query->query == NULL) { + status = ares_dns_record_duplicate_ex(&query->query, dnsrec); + if (status != ARES_SUCCESS) { ares_free(query); - callback(arg, ARES_ENOMEM, 0, NULL); - return ARES_ENOMEM; + callback(arg, status, 0, NULL); + return status; } ares_dns_record_set_id(query->query, id); diff --git a/test/ares-test-mock.cc b/test/ares-test-mock.cc index da196cd3..9961973d 100644 --- a/test/ares-test-mock.cc +++ b/test/ares-test-mock.cc @@ -197,6 +197,21 @@ TEST_P(MockUDPChannelTest, TruncationRetry) { EXPECT_EQ("{'www.google.com' aliases=[] addrs=[1.2.3.4]}", ss.str()); } +TEST_P(MockUDPChannelTest, UTF8BadName) { + DNSPacket reply; + reply.set_response().set_aa() + .add_question(new DNSQuestion("españa.icom.museum", T_A)) + .add_answer(new DNSARR("españa.icom.museum", 100, {2, 3, 4, 5})); + ON_CALL(server_, OnRequest("españa.icom.museum", T_A)) + .WillByDefault(SetReply(&server_, &reply)); + + HostResult result; + ares_gethostbyname(channel_, "españa.icom.museum", AF_INET, HostCallback, &result); + Process(); + EXPECT_TRUE(result.done_); + EXPECT_EQ(ARES_EBADNAME, result.status_); +} + static int sock_cb_count = 0; static int SocketConnectCallback(ares_socket_t fd, int type, void *data) { int rc = *(int*)data;