dns: add some additional error checks.

If the upper 2 bits of a label/pointer value are 10, treat that as an
error.

If a name is longer than 255 characters, treat that as an error.

This prevents some long loops with malformed packets, as found by Hardik
Shah.

diff --git a/nameser.h b/nameser.h
index eee52b5da471d0d83c5988646b0b7132361a2add..526c60cab3b94a86c27a1abffe577b459e6aa61b 100644 (file)
--- a/nameser.h
+++ b/nameser.h
@@ -293,8 +293,11 @@ typedef struct {
 /*
  * Defines for handling compressed domain names, EDNS0 labels, etc.
  */
-#define INDIR_MASK     0xc0    /* 11.... */
-#define EDNS0_MASK     0x40    /* 01.... */
+#define TYPE_MASK      0xc0    /* mask for the type bits of the item */
+#define TYPE_INDIR     0xc0    /* 11.... - pointer */
+#define TYPE_RESERVED  0x80    /* 10.... - reserved */
+#define TYPE_EDNS0     0x40    /* 01.... - EDNS(0) label */
+#define TYPE_LABEL     0x00    /* 00.... - regular label */
 #  define EDNS0_ELT_BITLABEL 0x01
 
 #endif /* !_NAMESER_H_ */

diff --git a/print-domain.c b/print-domain.c
index df10e967f5a8764859859a7759164867cca78bc7..74c71dba0ca8e41eac48b21575f99be5e0f91dfe 100644 (file)
--- a/print-domain.c
+++ b/print-domain.c
@@ -74,12 +74,15 @@ ns_nskip(netdissect_options *ndo,
        i = GET_U_1(cp);
        cp++;
        while (i) {
-               if ((i & INDIR_MASK) == INDIR_MASK)
+               switch (i & TYPE_MASK) {
+
+               case TYPE_INDIR:
                        return (cp + 1);
-               if ((i & INDIR_MASK) == EDNS0_MASK) {
+
+               case TYPE_EDNS0: {
                        int bitlen, bytelen;
 
-                       if ((i & ~INDIR_MASK) != EDNS0_ELT_BITLABEL)
+                       if ((i & ~TYPE_MASK) != EDNS0_ELT_BITLABEL)
                                return(NULL); /* unknown ELT */
                        if (!ND_TTEST_1(cp))
                                return (NULL);
@@ -88,8 +91,16 @@ ns_nskip(netdissect_options *ndo,
                        cp++;
                        bytelen = (bitlen + 7) / 8;
                        cp += bytelen;
-               } else
+               }
+               break;
+
+               case TYPE_RESERVED:
+                       return (NULL);
+
+               case TYPE_LABEL:
                        cp += i;
+                       break;
+               }
                if (!ND_TTEST_1(cp))
                        return (NULL);
                i = GET_U_1(cp);
@@ -140,9 +151,11 @@ labellen(netdissect_options *ndo,
        if (!ND_TTEST_1(cp))
                return(-1);
        i = GET_U_1(cp);
-       if ((i & INDIR_MASK) == EDNS0_MASK) {
+       switch (i & TYPE_MASK) {
+
+       case TYPE_EDNS0: {
                u_int bitlen, elt;
-               if ((elt = (i & ~INDIR_MASK)) != EDNS0_ELT_BITLABEL) {
+               if ((elt = (i & ~TYPE_MASK)) != EDNS0_ELT_BITLABEL) {
                        ND_PRINT("<ELT %d>", elt);
                        return(-1);
                }
@@ -151,8 +164,20 @@ labellen(netdissect_options *ndo,
                if ((bitlen = GET_U_1(cp + 1)) == 0)
                        bitlen = 256;
                return(((bitlen + 7) / 8) + 1);
-       } else
+       }
+
+       case TYPE_INDIR:
+       case TYPE_LABEL:
                return(i);
+
+       default:
+               /*
+                * TYPE_RESERVED, but we use default to suppress compiler
+                * warnings about falling out of the switch statement.
+                */
+               ND_PRINT("<BAD LABEL TYPE>");
+               return(-1);
+       }
 }
 
 /* print a <domain-name> */
@@ -165,6 +190,7 @@ fqdn_print(netdissect_options *ndo,
        int compress = 0;
        u_int elt;
        u_int offset, max_offset;
+       u_int name_chars = 0;
 
        if ((l = labellen(ndo, cp)) == (u_int)-1)
                return(NULL);
@@ -173,14 +199,16 @@ fqdn_print(netdissect_options *ndo,
        max_offset = (u_int)(cp - bp);
        i = GET_U_1(cp);
        cp++;
-       if ((i & INDIR_MASK) != INDIR_MASK) {
+       if ((i & TYPE_MASK) != TYPE_INDIR) {
                compress = 0;
                rp = cp + l;
        }
 
-       if (i != 0)
+       if (i != 0) {
                while (i && cp < ndo->ndo_snapend) {
-                       if ((i & INDIR_MASK) == INDIR_MASK) {
+                       switch (i & TYPE_MASK) {
+
+                       case TYPE_INDIR:
                                if (!compress) {
                                        rp = cp + 1;
                                        compress = 1;
@@ -204,16 +232,16 @@ fqdn_print(netdissect_options *ndo,
                                }
                                max_offset = offset;
                                cp = bp + offset;
-                               if ((l = labellen(ndo, cp)) == (u_int)-1)
-                                       return(NULL);
                                if (!ND_TTEST_1(cp))
                                        return(NULL);
                                i = GET_U_1(cp);
+                               if ((l = labellen(ndo, cp)) == (u_int)-1)
+                                       return(NULL);
                                cp++;
                                continue;
-                       }
-                       if ((i & INDIR_MASK) == EDNS0_MASK) {
-                               elt = (i & ~INDIR_MASK);
+
+                       case TYPE_EDNS0:
+                               elt = (i & ~TYPE_MASK);
                                switch(elt) {
                                case EDNS0_ELT_BITLABEL:
                                        if (blabel_print(ndo, cp) == NULL)
@@ -224,23 +252,41 @@ fqdn_print(netdissect_options *ndo,
                                        ND_PRINT("<ELT %u>", elt);
                                        return(NULL);
                                }
-                       } else {
-                               if (nd_printn(ndo, cp, l, ndo->ndo_snapend))
-                                       return(NULL);
+                               break;
+
+                       case TYPE_RESERVED:
+                               ND_PRINT("<BAD LABEL TYPE>");
+                               return(NULL);
+
+                       case TYPE_LABEL:
+                               if (name_chars + l <= MAXCDNAME) {
+                                       if (nd_printn(ndo, cp, l, ndo->ndo_snapend))
+                                               return(NULL);
+                               } else if (name_chars < MAXCDNAME) {
+                                       if (nd_printn(ndo, cp,
+                                           MAXCDNAME - name_chars, ndo->ndo_snapend))
+                                               return(NULL);
+                               }
+                               name_chars += l;
+                               break;
                        }
 
                        cp += l;
-                       ND_PRINT(".");
-                       if ((l = labellen(ndo, cp)) == (u_int)-1)
-                               return(NULL);
+                       if (name_chars <= MAXCDNAME)
+                               ND_PRINT(".");
+                       name_chars++;
                        if (!ND_TTEST_1(cp))
                                return(NULL);
                        i = GET_U_1(cp);
+                       if ((l = labellen(ndo, cp)) == (u_int)-1)
+                               return(NULL);
                        cp++;
                        if (!compress)
                                rp += l + 1;
                }
-       else
+               if (name_chars > MAXCDNAME)
+                       ND_PRINT("<DOMAIN NAME TOO LONG>");
+       } else
                ND_PRINT(".");
        return (rp);
 }

diff --git a/tests/TESTLIST b/tests/TESTLIST
index 124316bb4365920fb93ae713ff562f501927edd0..75e6394e6d687f99e9f6fdf2ae26fac4ea47c323 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -818,6 +818,9 @@ ptp_ethernet        ptp_ethernet.pcap       ptp_ethernet.out        -e
 # bad packets from Jason Xiaole
 ldp_tlv_print-oobr ldp_tlv_print-oobr.pcap ldp_tlv_print-oobr.out -v
 
+# bad packets from Hardik Shah
+dns-badlabel   dns-badlabel.pcap       dns-badlabel.out        -vv
+
 #someip tests
 someip1                someip1.pcap    someip1.out
 someip2                someip2.pcap    someip2.out

diff --git a/tests/dns-badlabel.out b/tests/dns-badlabel.out
new file mode 100644 (file)
index 0000000..de38379
--- /dev/null
+++ b/tests/dns-badlabel.out
@@ -0,0 +1,2 @@
+    1  10:04:02.000000 IP (tos 0x0, ttl 128, id 36039, offset 0, flags [none], proto UDP (17), length 63193)
+    156.118.17.235.53 > 156.118.27.229.500: [bad udp cksum 0xd8da -> 0x54de!] 51584 zoneRef NoChange*|$ [64259q] q: Type0 (Class 4347)? M-{.^AM-{^C.M-{.^AM-{^C.M-{.^@M-{.^AM-{^C.M-{.^AM-{^C.M-{.^CM-{^C^AM-{^C^AM-{^C^AM-{^C^AM-{^C^AM-{^C^AM-{^C^AM-{^C^AM-{^C^AM-{^C^AM-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-zM-^O.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^A<DOMAIN NAME TOO LONG>, q: Type769 (Class 64259)? ^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.<BAD LABEL TYPE> [|domain]

diff --git a/tests/dns-badlabel.pcap b/tests/dns-badlabel.pcap
new file mode 100644 (file)
index 0000000..2a3739d
Binary files /dev/null and b/tests/dns-badlabel.pcap differ