From 510f1de8a6add516b842a55750366944701d3d9a Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 19 Jan 2024 14:26:17 +0000 Subject: [PATCH] fix another message parsing regression The fix for CVE-2023-4408 introduced a regression in the message parser, which could cause a crash if an rdata type that can only occur in the question was found in another section. Use 'dns__message_putassociatedrdataset()' instead of 'dns__message_puttemprdataset()', because after calling the 'dns_rdatalist_tordataset()' function earlier the 'rdataset' is associated. --- lib/dns/message.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/lib/dns/message.c b/lib/dns/message.c index b0d5f16da7..b301819480 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -1250,6 +1250,7 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, isedns = false; issigzero = false; istsig = false; + found_rdataset = NULL; name = NULL; dns_message_gettempname(msg, &name); @@ -1394,10 +1395,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, * Then put the meta-class back into the finished rdata. */ rdata = newrdata(msg); - if (rdata == NULL) { - result = ISC_R_NOMEMORY; - goto cleanup; - } if (msg->opcode == dns_opcode_update && update(sectionid, rdclass)) { @@ -1549,7 +1546,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, * the question section, fail. */ if (dns_rdatatype_questiononly(rdtype)) { - dns_message_puttemprdataset(msg, &rdataset); DO_ERROR(DNS_R_FORMERR); } @@ -1600,18 +1596,17 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, /* Free the rdataset we used as the key */ dns__message_putassociatedrdataset(msg, &rdataset); - rdataset = found_rdataset; - result = ISC_R_SUCCESS; + rdataset = found_rdataset; if (!dns_rdatatype_issingleton(rdtype)) { break; } - dns_rdata_t *first; dns_rdatalist_fromrdataset(rdataset, &rdatalist); - first = ISC_LIST_HEAD(rdatalist->rdata); + dns_rdata_t *first = + ISC_LIST_HEAD(rdatalist->rdata); INSIST(first != NULL); if (dns_rdata_compare(rdata, first) != 0) { DO_ERROR(DNS_R_FORMERR); @@ -1656,7 +1651,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, dns_rcode_t ercode; msg->opt = rdataset; - rdataset = NULL; ercode = (dns_rcode_t)((msg->opt->ttl & DNS_MESSAGE_EDNSRCODE_MASK) >> 20); @@ -1667,7 +1661,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, msg->sig0 = rdataset; msg->sig0name = name; msg->sigstart = recstart; - rdataset = NULL; free_name = false; } else if (istsig) { msg->tsig = rdataset; @@ -1677,9 +1670,9 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, * Windows doesn't like TSIG names to be compressed. */ msg->tsigname->attributes.nocompress = true; - rdataset = NULL; free_name = false; } + rdataset = NULL; if (seen_problem) { if (free_name) { @@ -1688,8 +1681,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, free_name = false; } INSIST(!free_name); - - rdataset = NULL; } /* @@ -1711,6 +1702,9 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, } cleanup: + if (rdataset != NULL && rdataset != found_rdataset) { + dns__message_putassociatedrdataset(msg, &rdataset); + } if (free_name) { dns_message_puttempname(msg, &name); }