Skip to content

Commit

Permalink
CVE-2017-12995/Check for DNS compression pointers that don't point ba…
Browse files Browse the repository at this point in the history
…ckwards.

This is what BIND 9.11.0-P2 does; it not only detects pointers that
loop, as "point backwards" means "point before anything we've processed
so far, including what we're processing right now", so the pointer can't
point to itself (as that's what we're processing right now).

This fixes an infinite loop discovered by Forcepoint's security
researchers Otto Airamo & Antti Levomäki.

Add a test using the capture file supplied by the reporter(s).

Also, add some infinite-pointer-loop captures.

More checks should be done.  We might, for example, want to make sure
the upper 2 bits of the label length/pointer byte are 00 or 11, and that
if we encounter a pointer and jump backwards to what we think is a label
the label ends before the beginning of the last label we processed, to
make sure the pointer doesn't point backwards into the *middle* of a
label, and also make sure the entire name is <= 255 bytes long.
  • Loading branch information
guyharris authored and infrastation committed Sep 13, 2017
1 parent 866c602 commit 3a76fd7
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 18 deletions.
37 changes: 19 additions & 18 deletions print-domain.c
Expand Up @@ -151,15 +151,14 @@ ns_nprint(netdissect_options *ndo,
register u_int i, l;
register const u_char *rp = NULL;
register int compress = 0;
int chars_processed;
int elt;
int data_size = ndo->ndo_snapend - bp;
u_int offset, max_offset;

if ((l = labellen(ndo, cp)) == (u_int)-1)
return(NULL);
if (!ND_TTEST2(*cp, 1))
return(NULL);
chars_processed = 1;
max_offset = (u_int)(cp - bp);
if (((i = *cp++) & INDIR_MASK) != INDIR_MASK) {
compress = 0;
rp = cp + l;
Expand All @@ -174,24 +173,28 @@ ns_nprint(netdissect_options *ndo,
}
if (!ND_TTEST2(*cp, 1))
return(NULL);
cp = bp + (((i << 8) | *cp) & 0x3fff);
offset = (((i << 8) | *cp) & 0x3fff);
/*
* This must move backwards in the packet.
* No RFC explicitly says that, but BIND's
* name decompression code requires it,
* as a way of preventing infinite loops
* and other bad behavior, and it's probably
* what was intended (compress by pointing
* to domain name suffixes already seen in
* the packet).
*/
if (offset >= max_offset) {
ND_PRINT((ndo, "<BAD PTR>"));
return(NULL);
}
max_offset = offset;
cp = bp + offset;
if ((l = labellen(ndo, cp)) == (u_int)-1)
return(NULL);
if (!ND_TTEST2(*cp, 1))
return(NULL);
i = *cp++;
chars_processed++;

/*
* If we've looked at every character in
* the message, this pointer will make
* us look at some character again,
* which means we're looping.
*/
if (chars_processed >= data_size) {
ND_PRINT((ndo, "<LOOP>"));
return (NULL);
}
continue;
}
if ((i & INDIR_MASK) == EDNS0_MASK) {
Expand All @@ -212,14 +215,12 @@ ns_nprint(netdissect_options *ndo,
}

cp += l;
chars_processed += l;
ND_PRINT((ndo, "."));
if ((l = labellen(ndo, cp)) == (u_int)-1)
return(NULL);
if (!ND_TTEST2(*cp, 1))
return(NULL);
i = *cp++;
chars_processed++;
if (!compress)
rp += l + 1;
}
Expand Down
9 changes: 9 additions & 0 deletions tests/TESTLIST
Expand Up @@ -474,6 +474,7 @@ zephyr-oobr zephyr-oobr.pcap zephyr-oobr.out -vvv -e
isakmp-no-none-np isakmp-no-none-np.pcap isakmp-no-none-np.out -vvv -e
telnet-iac-check-oobr telnet-iac-check-oobr.pcap telnet-iac-check-oobr.out -vvv -e
resp_4_infiniteloop resp_4_infiniteloop.pcap resp_4_infiniteloop.out -vvv -e
dns_fwdptr dns_fwdptr.pcap dns_fwdptr.out -vvv -e

# RTP tests
# fuzzed pcap
Expand All @@ -483,3 +484,11 @@ rtp-seg-fault-2 rtp-seg-fault-2.pcap rtp-seg-fault-2.out -v -T rtp
# NFS tests
# fuzzed pcap
nfs-seg-fault-1 nfs-seg-fault-1.pcap nfs-seg-fault-1.out

# DNS infinite loop tests
#
# See http://marc.info/?l=tcpdump-workers&m=95552439022555
#
dns-zlip-1 dns-zlip-1.pcap dns-zlip-1.out
dns-zlip-2 dns-zlip-2.pcap dns-zlip-2.out
dns-zlip-3 dns-zlip-3.pcap dns-zlip-3.out
1 change: 1 addition & 0 deletions tests/dns-zlip-1.out
@@ -0,0 +1 @@
IP 10.0.0.1.1024 > 146.84.28.88.53: 60777 Type49159 (Class 49168)? <BAD PTR>[|domain]
Binary file added tests/dns-zlip-1.pcap
Binary file not shown.
1 change: 1 addition & 0 deletions tests/dns-zlip-2.out
@@ -0,0 +1 @@
IP 10.0.0.1.1024 > 146.84.28.88.53: 18992 Type49164 (Class 49168)? <BAD PTR>[|domain]
Binary file added tests/dns-zlip-2.pcap
Binary file not shown.
1 change: 1 addition & 0 deletions tests/dns-zlip-3.out
@@ -0,0 +1 @@
IP 10.0.0.1.1024 > 146.84.28.88.53: 65483 Type49164 (Class 49164)? thisleetostringwillcrashyourlittlenameserverforsurehahahahahah.<BAD PTR>[|domain]
Binary file added tests/dns-zlip-3.pcap
Binary file not shown.
2 changes: 2 additions & 0 deletions tests/dns_fwdptr.out
@@ -0,0 +1,2 @@
be:af:ca:ce:ff:ff > de:ad:be:ef:00:01, ethertype IPv4 (0x0800), length 63207: (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: [udp sum ok] 51584 zoneRef NoChange*|$ [64259q] q: Type507 (Class 769)? M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{^C.M-{.^AM-{ .M-{^C^A.<BAD PTR>[|domain]
Binary file added tests/dns_fwdptr.pcap
Binary file not shown.

1 comment on commit 3a76fd7

@raybellis
Copy link

@raybellis raybellis commented on 3a76fd7 Sep 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RFC 1035 has an implicit requirement for "backwards pointers" in §4.1.4 (my emphasis)

In order to reduce the size of messages, the domain system utilizes a
compression scheme which eliminates the repetition of domain names in a
message. In this scheme, an entire domain name or a list of labels at
the end of a domain name is replaced with a pointer to a prior occurance
of the same name.

Arguably, a compression pointer also cannot contain a value of less than 12, since that then points into the message header and not to a "name".

Please sign in to comment.