DNS: fix response name length logic

In some cases where the length would be calculated as 0 we'd loop until
we'd hit our loop limit.

Update name logic everywhere.

diff --git a/src/app-layer-dns-common.c b/src/app-layer-dns-common.c
index feb3db99cfa60a708bbbe61ba7d1f21b1c40cd42..3cf0317cae7812073b8186c64c0f2afba54c431a 100644 (file)
--- a/src/app-layer-dns-common.c
+++ b/src/app-layer-dns-common.c
@@ -435,18 +435,20 @@ static uint16_t DNSResponseGetNameByOffset(const uint8_t * const input, const ui
         }
         qdata++;
 
-        if (length > 0) {
-            if (input + input_len < qdata + length) {
-                SCLogDebug("input buffer too small for domain of len %u", length);
-                goto insufficient_data;
-            }
-            //PrintRawDataFp(stdout, qdata, length);
+        if (length == 0) {
+            break;
+        }
 
-            if ((size_t)(fqdn_offset + length + 1) < fqdn_size) {
-                memcpy(fqdn + fqdn_offset, qdata, length);
-                fqdn_offset += length;
-                fqdn[fqdn_offset++] = '.';
-            }
+        if (input + input_len < qdata + length) {
+            SCLogDebug("input buffer too small for domain of len %u", length);
+            goto insufficient_data;
+        }
+        //PrintRawDataFp(stdout, qdata, length);
+
+        if ((size_t)(fqdn_offset + length + 1) < fqdn_size) {
+            memcpy(fqdn + fqdn_offset, qdata, length);
+            fqdn_offset += length;
+            fqdn[fqdn_offset++] = '.';
         }
         qdata += length;
 
@@ -681,7 +683,7 @@ const uint8_t *DNSReponseParse(DNSState *dns_state, const DNSHeader * const dns_
 
                 uint8_t pmail[DNS_MAX_SIZE];
                 uint16_t pmail_len = 0;
-
+                SCLogDebug("getting pmail");
                 if ((pmail_len = DNSResponseGetNameByOffset(input, input_len,
                                 sdata - input, pmail, sizeof(pmail))) == 0)
                 {
@@ -691,6 +693,8 @@ const uint8_t *DNSReponseParse(DNSState *dns_state, const DNSHeader * const dns_
 #endif
                     goto insufficient_data;
                 }
+                SCLogDebug("pmail_len %u", pmail_len);
+                //PrintRawDataFp(stdout, (uint8_t *)pmail, pmail_len);
 
                 const uint8_t *tdata = SkipDomain(input, input_len, sdata);
                 if (tdata == NULL) {

diff --git a/src/app-layer-dns-udp.c b/src/app-layer-dns-udp.c
index 71d552f77a8dae744859c64980d100aabfce2c7a..d68791e386f8e6357e02cce371d7e74d5b4f6330 100644 (file)
--- a/src/app-layer-dns-udp.c
+++ b/src/app-layer-dns-udp.c
@@ -95,21 +95,23 @@ static int DNSUDPRequestParse(Flow *f, void *dstate,
 
             data++;
 
-            if (length > 0) {
-                if (input + input_len < data + length) {
-                    SCLogDebug("input buffer too small for domain of len %u", length);
-                    goto insufficient_data;
-                }
-                //PrintRawDataFp(stdout, data, qry->length);
-
-                if ((size_t)(fqdn_offset + length + 1) < sizeof(fqdn)) {
-                    memcpy(fqdn + fqdn_offset, data, length);
-                    fqdn_offset += length;
-                    fqdn[fqdn_offset++] = '.';
-                } else {
-                    /** \todo set event? */
-                    goto insufficient_data;
-                }
+            if (length == 0) {
+                break;
+            }
+
+            if (input + input_len < data + length) {
+                SCLogDebug("input buffer too small for domain of len %u", length);
+                goto insufficient_data;
+            }
+            //PrintRawDataFp(stdout, data, qry->length);
+
+            if ((size_t)(fqdn_offset + length + 1) < sizeof(fqdn)) {
+                memcpy(fqdn + fqdn_offset, data, length);
+                fqdn_offset += length;
+                fqdn[fqdn_offset++] = '.';
+            } else {
+                /** \todo set event? */
+                goto insufficient_data;
             }
 
             data += length;
@@ -173,7 +175,7 @@ static int DNSUDPResponseParse(Flow *f, void *dstate,
     }
 
     DNSHeader *dns_header = (DNSHeader *)input;
-    SCLogDebug("DNS %p", dns_header);
+    SCLogDebug("DNS %p %04x %04x", dns_header, ntohs(dns_header->tx_id), dns_header->flags);
 
     DNSTransaction *tx = NULL;
     int found = 0;
@@ -186,6 +188,8 @@ static int DNSUDPResponseParse(Flow *f, void *dstate,
     if (DNSValidateResponseHeader(dns_state, dns_header) < 0)
         goto bad_data;
 
+    SCLogDebug("queries %04x", ntohs(dns_header->questions));
+
     uint16_t q;
     const uint8_t *data = input + sizeof(DNSHeader);
     for (q = 0; q < ntohs(dns_header->questions); q++) {
@@ -202,18 +206,19 @@ static int DNSUDPResponseParse(Flow *f, void *dstate,
             uint8_t length = *data;
             data++;
 
-            if (length > 0) {
-                if (input + input_len < data + length) {
-                    SCLogDebug("input buffer too small for domain of len %u", length);
-                    goto insufficient_data;
-                }
-                //PrintRawDataFp(stdout, data, length);
-
-                if ((size_t)(fqdn_offset + length + 1) < sizeof(fqdn)) {
-                    memcpy(fqdn + fqdn_offset, data, length);
-                    fqdn_offset += length;
-                    fqdn[fqdn_offset++] = '.';
-                }
+            if (length == 0)
+                break;
+
+            if (input + input_len < data + length) {
+                SCLogDebug("input buffer too small for domain of len %u", length);
+                goto insufficient_data;
+            }
+            //PrintRawDataFp(stdout, data, length);
+
+            if ((size_t)(fqdn_offset + length + 1) < sizeof(fqdn)) {
+                memcpy(fqdn + fqdn_offset, data, length);
+                fqdn_offset += length;
+                fqdn[fqdn_offset++] = '.';
             }
 
             data += length;
@@ -242,6 +247,7 @@ static int DNSUDPResponseParse(Flow *f, void *dstate,
         data += sizeof(DNSQueryTrailer);
     }
 
+    SCLogDebug("answer_rr %04x", ntohs(dns_header->answer_rr));
     for (q = 0; q < ntohs(dns_header->answer_rr); q++) {
         data = DNSReponseParse(dns_state, dns_header, q, DNS_LIST_ANSWER,
                 input, input_len, data);
@@ -250,6 +256,7 @@ static int DNSUDPResponseParse(Flow *f, void *dstate,
         }
     }
 
+    SCLogDebug("authority_rr %04x", ntohs(dns_header->authority_rr));
     for (q = 0; q < ntohs(dns_header->authority_rr); q++) {
         data = DNSReponseParse(dns_state, dns_header, q, DNS_LIST_AUTHORITY,
                 input, input_len, data);
@@ -346,11 +353,53 @@ void RegisterDNSUDPParsers(void) {
         SCLogInfo("Parsed disabled for %s protocol. Protocol detection"
                   "still on.", proto_name);
     }
+#ifdef UNITTESTS
+    AppLayerParserRegisterUnittests(ALPROTO_DNS_UDP, DNSUDPParserRegisterTests);
+#endif
 }
 
 /* UNITTESTS */
 #ifdef UNITTESTS
+#include "util-unittest-helper.h"
+
+static int DNSUDPParserTest01 (void) {
+    int result = 0;
+    /* query: abcdefghijk.com
+     * TTL: 86400
+     * serial 20130422 refresh 28800 retry 7200 exp 604800 min ttl 86400
+     * ns, hostmaster */
+    uint8_t buf[] = { 0x00, 0x3c, 0x85, 0x00, 0x00, 0x01, 0x00, 0x00,
+                      0x00, 0x01, 0x00, 0x00, 0x0b, 0x61, 0x62, 0x63,
+                      0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6a, 0x6b,
+                      0x03, 0x63, 0x6f, 0x6d, 0x00, 0x00, 0x0f, 0x00,
+                      0x01, 0x00, 0x00, 0x06, 0x00, 0x01, 0x00, 0x01,
+                      0x51, 0x80, 0x00, 0x25, 0x02, 0x6e, 0x73, 0x00,
+                      0x0a, 0x68, 0x6f, 0x73, 0x74, 0x6d, 0x61, 0x73,
+                      0x74, 0x65, 0x72, 0xc0, 0x2f, 0x01, 0x33, 0x2a,
+                      0x76, 0x00, 0x00, 0x70, 0x80, 0x00, 0x00, 0x1c,
+                      0x20, 0x00, 0x09, 0x3a, 0x80, 0x00, 0x01, 0x51,
+                      0x80};
+    size_t buflen = sizeof(buf);
+    Flow *f = NULL;
+
+    f = UTHBuildFlow(AF_INET, "1.2.3.4", "1.2.3.5", 1024, 53);
+    if (f == NULL)
+        goto end;
+    f->proto = IPPROTO_UDP;
+    f->alproto = ALPROTO_DNS_UDP;
+    f->alstate = DNSStateAlloc();
+
+    int r = DNSUDPResponseParse(f, f->alstate, NULL, buf, buflen, NULL, NULL);
+    if (r != 1)
+        goto end;
+
+    result = 1;
+end:
+    UTHFreeFlow(f);
+    return (result);
+}
+
 void DNSUDPParserRegisterTests(void) {
-//     UtRegisterTest("DNSUDPParserTest01", DNSUDPParserTest01, 1);
+       UtRegisterTest("DNSUDPParserTest01", DNSUDPParserTest01, 1);
 }
 #endif