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.

diff --git a/addrtoname.c b/addrtoname.c
index 90ae5c5b1422ac6d155f3887c37bce604d8c914e..d98929cd6b3a3dffa10a6ee86fdd795149b81480 100644 (file)
--- 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 0ddc63e39a8fd53584e1ed60a8ef7b70967aeb6b..5f0dc0eb4fd7a9af6cbeef5a377eb338fc12e9c9 100644 (file)
--- 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 (file)
index 0000000..7d8613b
--- /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 (file)
index 0000000..9f13f63
Binary files /dev/null and b/tests/hoobr_lookup_nsap.pcap differ