Fix assertion failure in nslookup/dig/mdig when message has multiple SIG(0) options.

When parsing message with DNS_MESSAGE_BESTEFFORT (used exclusively in
tools, never in named itself) if we hit an invalid SIG(0) in wrong
place we continue parsing the message, and put the sig0 in msg->sig0.
If we then hit another sig0 in a proper place we see that msg->sig0
is already 'taken' and we don't free name and rdataset, and we don't
set seen_problem. This causes an assertion failure.
This fixes that issue by setting seen_problem if we hit second sig0,
tsig or opt, which causes name and rdataset to be always freed.

(cherry picked from commit 51a55ddbb73f8707de3d1b8cda15c8f61585bacb)

diff --git a/lib/dns/message.c b/lib/dns/message.c
index d03fc8d9c5abada9bca157fcbcabae52ed192d1a..f7682db2639ccf7c55c8b44a1dbdd32664fc0f99 100644 (file)
--- a/lib/dns/message.c
+++ b/lib/dns/message.c
@@ -1238,7 +1238,7 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
        dns_namelist_t *section;
        bool free_name = false, free_rdataset = false;
        bool preserve_order, best_effort, seen_problem;
-       bool issigzero;
+       bool isedns, issigzero, istsig;
 
        preserve_order = ((options & DNS_MESSAGEPARSE_PRESERVEORDER) != 0);
        best_effort = ((options & DNS_MESSAGEPARSE_BESTEFFORT) != 0);
@@ -1253,6 +1253,9 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                skip_name_search = false;
                skip_type_search = false;
                free_rdataset = false;
+               isedns = false;
+               issigzero = false;
+               istsig = false;
 
                name = isc_mempool_get(msg->namepool);
                if (name == NULL)
@@ -1334,11 +1337,13 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                         */
                        if (sectionid != DNS_SECTION_ADDITIONAL ||
                            rdclass != dns_rdataclass_any ||
-                           count != msg->counts[sectionid]  - 1)
+                           count != msg->counts[sectionid]  - 1) {
                                DO_ERROR(DNS_R_BADTSIG);
-                       msg->sigstart = recstart;
-                       skip_name_search = true;
-                       skip_type_search = true;
+                       } else {
+                               skip_name_search = true;
+                               skip_type_search = true;
+                               istsig = true;
+                       }
                } else if (rdtype == dns_rdatatype_opt) {
                        /*
                         * The name of an OPT record must be ".", it
@@ -1347,10 +1352,13 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                         */
                        if (!dns_name_equal(dns_rootname, name) ||
                            sectionid != DNS_SECTION_ADDITIONAL ||
-                           msg->opt != NULL)
+                           msg->opt != NULL) {
                                DO_ERROR(DNS_R_FORMERR);
-                       skip_name_search = true;
-                       skip_type_search = true;
+                       } else {
+                               skip_name_search = true;
+                               skip_type_search = true;
+                               isedns = true;
+                       }
                } else if (rdtype == dns_rdatatype_tkey) {
                        /*
                         * A TKEY must be in the additional section if this
@@ -1422,7 +1430,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                if (result != ISC_R_SUCCESS)
                        goto cleanup;
                rdata->rdclass = rdclass;
-               issigzero = false;
                if (rdtype == dns_rdatatype_rrsig &&
                    rdata->flags == 0) {
                        covers = dns_rdata_covers(rdata);
@@ -1433,12 +1440,13 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                        covers = dns_rdata_covers(rdata);
                        if (covers == 0) {
                                if (sectionid != DNS_SECTION_ADDITIONAL ||
-                                   count != msg->counts[sectionid]  - 1)
+                                   count != msg->counts[sectionid]  - 1) {
                                        DO_ERROR(DNS_R_BADSIG0);
-                               msg->sigstart = recstart;
-                               skip_name_search = true;
-                               skip_type_search = true;
-                               issigzero = true;
+                               } else {
+                                       skip_name_search = true;
+                                       skip_type_search = true;
+                                       issigzero = true;
+                               }
                        } else {
                                if (msg->rdclass != dns_rdataclass_any &&
                                    msg->rdclass != rdclass)
@@ -1464,10 +1472,7 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                 */
                if (preserve_order || msg->opcode == dns_opcode_update ||
                    skip_name_search) {
-                       if (rdtype != dns_rdatatype_opt &&
-                           rdtype != dns_rdatatype_tsig &&
-                           !issigzero)
-                       {
+                       if (!isedns && !istsig && !issigzero) {
                                ISC_LIST_APPEND(*section, name, link);
                                free_name = false;
                        }
@@ -1560,10 +1565,7 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                                      == ISC_R_SUCCESS);
                        dns_rdataset_setownercase(rdataset, name);
 
-                       if (rdtype != dns_rdatatype_opt &&
-                           rdtype != dns_rdatatype_tsig &&
-                           !issigzero)
-                       {
+                       if (!isedns && !istsig && !issigzero) {
                                ISC_LIST_APPEND(name->list, rdataset, link);
                                free_rdataset = false;
                        }
@@ -1595,7 +1597,7 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                 * already set if best-effort parsing is enabled otherwise
                 * there will only be at most one of each.
                 */
-               if (rdtype == dns_rdatatype_opt && msg->opt == NULL) {
+               if (isedns) {
                        dns_rcode_t ercode;
 
                        msg->opt = rdataset;
@@ -1607,16 +1609,20 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t *dctx,
                        msg->rcode |= ercode;
                        isc_mempool_put(msg->namepool, name);
                        free_name = false;
-               } else if (issigzero && msg->sig0 == NULL) {
+               } else if (issigzero) {
                        msg->sig0 = rdataset;
                        msg->sig0name = name;
+                       msg->sigstart = recstart;
                        rdataset = NULL;
                        free_rdataset = false;
                        free_name = false;
-               } else if (rdtype == dns_rdatatype_tsig && msg->tsig == NULL) {
+               } else if (istsig) {
                        msg->tsig = rdataset;
                        msg->tsigname = name;
-                       /* Windows doesn't like TSIG names to be compressed. */
+                       msg->sigstart = recstart;
+                       /*
+                        * Windows doesn't like TSIG names to be compressed.
+                        */
                        msg->tsigname->attributes |= DNS_NAMEATTR_NOCOMPRESS;
                        rdataset = NULL;
                        free_rdataset = false;