From: Guy Harris Date: Sun, 15 Nov 2020 04:34:25 +0000 (-0800) Subject: dns: add some additional error checks. X-Git-Tag: tcpdump-4.99-bp~58 X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/92d636a906d450f9bd344ee312cfa9c88c3d2bd6 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 eee52b5d..526c60ca 100644 --- 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 df10e967..74c71dba 100644 --- 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); 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(""); + return(-1); + } } /* print a */ @@ -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); return(NULL); } - } else { - if (nd_printn(ndo, cp, l, ndo->ndo_snapend)) - return(NULL); + break; + + case TYPE_RESERVED: + ND_PRINT(""); + 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(""); + } else ND_PRINT("."); return (rp); } diff --git a/tests/TESTLIST b/tests/TESTLIST index 124316bb..75e6394e 100644 --- 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 index 00000000..de38379b --- /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, 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. [|domain] diff --git a/tests/dns-badlabel.pcap b/tests/dns-badlabel.pcap new file mode 100644 index 00000000..2a3739d1 Binary files /dev/null and b/tests/dns-badlabel.pcap differ