summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGuy Harris <guy@alum.mit.edu>2017-02-10 19:52:29 -0800
committerDenis Ovsienko <denis@ovsienko.info>2017-09-13 12:25:44 +0100
commit777edc563aacdaff66a0b829cecd2ccb09a10404 (patch)
tree55b97475efe571a2df07188cd12f3db39d9f8454
parent3a76fd7c95fced2c2f8c8148a9055c3a542eff29 (diff)
downloadtcpdump-777edc563aacdaff66a0b829cecd2ccb09a10404.tar.gz
Further fix the fix to CVE-2017-5485.
1) Take the length of the NSAP into account. Otherwise, if, in our search of the hash table, we come across a byte string that's shorter than the string we're looking for, we'll search past the end of the string in the hash table. 2) The first byte of the byte string in the table is the length of the NSAP, with the byte *after* that being the first byte of the NSAP, but the first byte of the byte string passed into lookup_nsap() is the first byte of the NSAP, with the length passed in as a separate argument. Do the comparison correctly. This fixes a vulnerability discovered by Kamil Frankowicz. Add a test using the capture file supplied by the reporter(s). While we're at it, clean up the fix to lookup_bytestring(): 1) Get rid of an unused structure member and an unused #define. 2) Get rid of an incorrect "+ 1" when calculating the size of the byte array to allocate - that was left over from the NSAP table, where the length was guaranteed to fit in 1 byte and we used the first byte of the array to hold the length of the rest of the array.
-rw-r--r--addrtoname.c10
-rw-r--r--tests/TESTLIST1
-rw-r--r--tests/hoobr_lookup_nsap.out23
-rw-r--r--tests/hoobr_lookup_nsap.pcapbin0 -> 1096 bytes
4 files changed, 28 insertions, 6 deletions
diff --git a/addrtoname.c b/addrtoname.c
index 90ae5c5b..d98929cd 100644
--- a/addrtoname.c
+++ b/addrtoname.c
@@ -150,8 +150,6 @@ struct enamemem {
u_short e_addr2;
const char *e_name;
u_char *e_nsap; /* used only for nsaptable[] */
-#define e_bs e_nsap /* for bytestringtable */
- size_t e_namelen; /* for bytestringtable */
struct enamemem *e_nxt;
};
@@ -425,7 +423,7 @@ lookup_bytestring(netdissect_options *ndo, register const u_char *bs,
tp->bs_addr1 = j;
tp->bs_addr2 = k;
- tp->bs_bytes = (u_char *) calloc(1, nlen + 1);
+ tp->bs_bytes = (u_char *) calloc(1, nlen);
if (tp->bs_bytes == NULL)
(*ndo->ndo_error)(ndo, "lookup_bytestring: calloc");
@@ -459,11 +457,11 @@ lookup_nsap(netdissect_options *ndo, register const u_char *nsap,
tp = &nsaptable[(i ^ j) & (HASHNAMESIZE-1)];
while (tp->e_nxt)
- if (tp->e_addr0 == i &&
+ if (nsap_length == tp->e_nsap[0] &&
+ tp->e_addr0 == i &&
tp->e_addr1 == j &&
tp->e_addr2 == k &&
- tp->e_nsap[0] == nsap_length &&
- memcmp((const char *)&(nsap[1]),
+ memcmp((const char *)nsap,
(char *)&(tp->e_nsap[1]), nsap_length) == 0)
return tp;
else
diff --git a/tests/TESTLIST b/tests/TESTLIST
index 0ddc63e3..5f0dc0eb 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -456,6 +456,7 @@ hoobr_juniper2 hoobr_juniper2.pcap hoobr_juniper2.out
hoobr_juniper3 hoobr_juniper3.pcap hoobr_juniper3.out
hoobr_parse_field hoobr_parse_field.pcap hoobr_parse_field.out
hoobr_chdlc_print hoobr_chdlc_print.pcap hoobr_chdlc_print.out
+hoobr_lookup_nsap hoobr_lookup_nsap.pcap hoobr_lookup_nsap.out
# bad packets from Wilfried Kirsch
slip-bad-direction slip-bad-direction.pcap slip-bad-direction.out -ve
diff --git a/tests/hoobr_lookup_nsap.out b/tests/hoobr_lookup_nsap.out
new file mode 100644
index 00000000..7d8613b0
--- /dev/null
+++ b/tests/hoobr_lookup_nsap.out
@@ -0,0 +1,23 @@
+30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
+ 0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
+ 0x0010: 3030 3030 3030 000000
+30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
+ 0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
+ 0x0010: 3030 3030 3030 000000
+30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
+ 0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
+ 0x0010: 3030 3030 3030 000000
+30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
+ 0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
+ 0x0010: 3030 3030 3030 000000
+30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
+ 0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
+ 0x0010: 3030 3030 3030 000000
+30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
+ 0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
+ 0x0010: 3030 3030 3030 000000
+CLNP, 30.0000.0000.0000 > 30.3030, unknown (16), length 808464417
+30:30:30:30:30:30 > 30:30:30:30:30:30, ethertype Unknown (0x3030), length 808464432:
+ 0x0000: 3030 3030 3030 3030 3030 3030 3030 3030 0000000000000000
+ 0x0010: 3030 3030 3030 000000
+CLNP, 30.0000.0000.0000 > 30.3030, unknown (16), length 808464417
diff --git a/tests/hoobr_lookup_nsap.pcap b/tests/hoobr_lookup_nsap.pcap
new file mode 100644
index 00000000..9f13f63b
--- /dev/null
+++ b/tests/hoobr_lookup_nsap.pcap
Binary files differ