CVE-2017-12897/ISO CLNS: Use ND_TTEST() for the bounds checks in isoclns_print().

This fixes a buffer over-read discovered by Kamil Frankowicz.

Don't pass the remaining caplen - that's too hard to get right, and we
were getting it wrong in at least one case; just use ND_TTEST().

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

diff --git a/netdissect.h b/netdissect.h
index 26d6d47b3d1260d5da05ff481df3f25954977a7c..9060d1500ead9c017cd27d0afb96f1966ead0de1 100644 (file)
--- a/netdissect.h
+++ b/netdissect.h
@@ -512,7 +512,7 @@ extern void ipx_netbios_print(netdissect_options *, const u_char *, u_int);
 extern void ipx_print(netdissect_options *, const u_char *, u_int);
 extern void isakmp_print(netdissect_options *, const u_char *, u_int, const u_char *);
 extern void isakmp_rfc3948_print(netdissect_options *, const u_char *, u_int, const u_char *);
-extern void isoclns_print(netdissect_options *, const u_char *, u_int, u_int);
+extern void isoclns_print(netdissect_options *, const u_char *, u_int);
 extern void krb_print(netdissect_options *, const u_char *);
 extern void l2tp_print(netdissect_options *, const u_char *, u_int);
 extern void lane_print(netdissect_options *, const u_char *, u_int, u_int);

diff --git a/print-atm.c b/print-atm.c
index 7c12262c9ea433fadc4f34e491917ac0b830615b..12ada49df75934f340858a7ea2dc7db4d0dd6309 100644 (file)
--- a/print-atm.c
+++ b/print-atm.c
@@ -262,7 +262,7 @@ atm_if_print(netdissect_options *ndo,
         if (*p == LLC_UI) {
             if (ndo->ndo_eflag)
                 ND_PRINT((ndo, "CNLPID "));
-            isoclns_print(ndo, p + 1, length - 1, caplen - 1);
+            isoclns_print(ndo, p + 1, length - 1);
             return hdrlen;
         }
 

diff --git a/print-chdlc.c b/print-chdlc.c
index ca96cc5060387f0dc58d5a7d2b4ec636e02af46f..24acfbd2e86c94ed85983d01181740831fa1f1e1 100644 (file)
--- a/print-chdlc.c
+++ b/print-chdlc.c
@@ -97,9 +97,9 @@ chdlc_print(netdissect_options *ndo, register const u_char *p, u_int length)
                 if (*(p+1) == 0x81 ||
                     *(p+1) == 0x82 ||
                     *(p+1) == 0x83)
-                    isoclns_print(ndo, p + 1, length - 1, ndo->ndo_snapend - p - 1);
+                    isoclns_print(ndo, p + 1, length - 1);
                 else
-                    isoclns_print(ndo, p, length, ndo->ndo_snapend - p);
+                    isoclns_print(ndo, p, length);
                 break;
        default:
                 if (!ndo->ndo_eflag)

diff --git a/print-ether.c b/print-ether.c
index 57c07ce935182931692fc98f62a40386159c0b37..33a135e0c373fe8407e1f1511dd9baf2145aae50 100644 (file)
--- a/print-ether.c
+++ b/print-ether.c
@@ -367,7 +367,7 @@ ethertype_print(netdissect_options *ndo,
                        ND_PRINT((ndo, " [|osi]"));
                        return (1);
                }
-               isoclns_print(ndo, p + 1, length - 1, caplen - 1);
+               isoclns_print(ndo, p + 1, length - 1);
                return(1);
 
        case ETHERTYPE_PPPOED:

diff --git a/print-fr.c b/print-fr.c
index dad8173c7e23bda5b22852b718eb3761b6db5362..f1a8b21e5051e6c69eabd1709dd0dae162fea295 100644 (file)
--- a/print-fr.c
+++ b/print-fr.c
@@ -329,7 +329,7 @@ fr_print(netdissect_options *ndo,
        case NLPID_CLNP:
        case NLPID_ESIS:
        case NLPID_ISIS:
-               isoclns_print(ndo, p - 1, length + 1, ndo->ndo_snapend - p + 1); /* OSI printers need the NLPID field */
+               isoclns_print(ndo, p - 1, length + 1); /* OSI printers need the NLPID field */
                break;
 
        case NLPID_SNAP:

diff --git a/print-gre.c b/print-gre.c
index 4664bd3f195c08c56a401db317424cc735cb816a..47a000cf5fc4686085076a0457caef7c6415cef7 100644 (file)
--- a/print-gre.c
+++ b/print-gre.c
@@ -227,7 +227,7 @@ gre_print_0(netdissect_options *ndo, const u_char *bp, u_int length)
                atalk_print(ndo, bp, len);
                break;
        case ETHERTYPE_GRE_ISO:
-               isoclns_print(ndo, bp, len, ndo->ndo_snapend - bp);
+               isoclns_print(ndo, bp, len);
                break;
        case ETHERTYPE_TEB:
                ether_print(ndo, bp, len, ndo->ndo_snapend - bp, NULL, NULL);

diff --git a/print-isoclns.c b/print-isoclns.c
index a3583e77b4cd51f1bd95d82630351aa1e25bb003..95339a0adc0ca5d48a2847553fab33beefe8ce39 100644 (file)
--- a/print-isoclns.c
+++ b/print-isoclns.c
@@ -670,10 +670,9 @@ struct isis_tlv_lsp {
 #define ISIS_PSNP_HEADER_SIZE (sizeof(struct isis_psnp_header))
 
 void
-isoclns_print(netdissect_options *ndo,
-              const uint8_t *p, u_int length, u_int caplen)
+isoclns_print(netdissect_options *ndo, const uint8_t *p, u_int length)
 {
-       if (caplen <= 1) { /* enough bytes on the wire ? */
+       if (!ND_TTEST(*p)) { /* enough bytes on the wire ? */
                ND_PRINT((ndo, "|OSI"));
                return;
        }
@@ -685,7 +684,7 @@ isoclns_print(netdissect_options *ndo,
 
        case NLPID_CLNP:
                if (!clnp_print(ndo, p, length))
-                       print_unknown_data(ndo, p, "\n\t", caplen);
+                       print_unknown_data(ndo, p, "\n\t", length);
                break;
 
        case NLPID_ESIS:
@@ -694,7 +693,7 @@ isoclns_print(netdissect_options *ndo,
 
        case NLPID_ISIS:
                if (!isis_print(ndo, p, length))
-                       print_unknown_data(ndo, p, "\n\t", caplen);
+                       print_unknown_data(ndo, p, "\n\t", length);
                break;
 
        case NLPID_NULLNS:
@@ -721,8 +720,8 @@ isoclns_print(netdissect_options *ndo,
                if (!ndo->ndo_eflag)
                        ND_PRINT((ndo, "OSI NLPID 0x%02x unknown", *p));
                ND_PRINT((ndo, "%slength: %u", ndo->ndo_eflag ? "" : ", ", length));
-               if (caplen > 1)
-                       print_unknown_data(ndo, p, "\n\t", caplen);
+               if (length > 1)
+                       print_unknown_data(ndo, p, "\n\t", length);
                break;
        }
 }

diff --git a/print-juniper.c b/print-juniper.c
index e4ee11c0b7f3c8b91183f6b75f24e3a5921972a3..9a258d9ae2de1a2cee6bb48fb2232524769f0a88 100644 (file)
--- a/print-juniper.c
+++ b/print-juniper.c
@@ -793,7 +793,7 @@ juniper_mlppp_print(netdissect_options *ndo,
             mpls_print(ndo, p, l2info.length);
             return l2info.header_len;
         case JUNIPER_LSQ_L3_PROTO_ISO:
-            isoclns_print(ndo, p, l2info.length, l2info.caplen);
+            isoclns_print(ndo, p, l2info.length);
             return l2info.header_len;
         default:
             break;
@@ -848,7 +848,7 @@ juniper_mfr_print(netdissect_options *ndo,
                 mpls_print(ndo, p, l2info.length);
                 return l2info.header_len;
             case JUNIPER_LSQ_L3_PROTO_ISO:
-                isoclns_print(ndo, p, l2info.length, l2info.caplen);
+                isoclns_print(ndo, p, l2info.length);
                 return l2info.header_len;
             default:
                 break;
@@ -861,13 +861,13 @@ juniper_mfr_print(netdissect_options *ndo,
             ND_PRINT((ndo, "Bundle-ID %u, ", l2info.bundle));
         switch (l2info.proto) {
         case (LLCSAP_ISONS<<8 | LLCSAP_ISONS):
-            isoclns_print(ndo, p + 1, l2info.length - 1, l2info.caplen - 1);
+            isoclns_print(ndo, p + 1, l2info.length - 1);
             break;
         case (LLC_UI<<8 | NLPID_Q933):
         case (LLC_UI<<8 | NLPID_IP):
         case (LLC_UI<<8 | NLPID_IP6):
             /* pass IP{4,6} to the OSI layer for proper link-layer printing */
-            isoclns_print(ndo, p - 1, l2info.length + 1, l2info.caplen + 1);
+            isoclns_print(ndo, p - 1, l2info.length + 1);
             break;
         default:
             ND_PRINT((ndo, "unknown protocol 0x%04x, length %u", l2info.proto, l2info.length));
@@ -896,13 +896,13 @@ juniper_mlfr_print(netdissect_options *ndo,
         switch (l2info.proto) {
         case (LLC_UI):
         case (LLC_UI<<8):
-            isoclns_print(ndo, p, l2info.length, l2info.caplen);
+            isoclns_print(ndo, p, l2info.length);
             break;
         case (LLC_UI<<8 | NLPID_Q933):
         case (LLC_UI<<8 | NLPID_IP):
         case (LLC_UI<<8 | NLPID_IP6):
             /* pass IP{4,6} to the OSI layer for proper link-layer printing */
-            isoclns_print(ndo, p - 1, l2info.length + 1, l2info.caplen + 1);
+            isoclns_print(ndo, p - 1, l2info.length + 1);
             break;
         default:
             ND_PRINT((ndo, "unknown protocol 0x%04x, length %u", l2info.proto, l2info.length));
@@ -949,7 +949,7 @@ juniper_atm1_print(netdissect_options *ndo,
         }
 
         if (p[0] == 0x03) { /* Cisco style NLPID encaps ? */
-            isoclns_print(ndo, p + 1, l2info.length - 1, l2info.caplen - 1);
+            isoclns_print(ndo, p + 1, l2info.length - 1);
             /* FIXME check if frame was recognized */
             return l2info.header_len;
         }
@@ -1004,7 +1004,7 @@ juniper_atm2_print(netdissect_options *ndo,
         }
 
         if (p[0] == 0x03) { /* Cisco style NLPID encaps ? */
-            isoclns_print(ndo, p + 1, l2info.length - 1, l2info.caplen - 1);
+            isoclns_print(ndo, p + 1, l2info.length - 1);
             /* FIXME check if frame was recognized */
             return l2info.header_len;
         }

diff --git a/print-llc.c b/print-llc.c
index 6bdf599846ed6750f165811fcff122e83127d828..be8886abdd4a11b48fe2eb0cb410c89484fccd6a 100644 (file)
--- a/print-llc.c
+++ b/print-llc.c
@@ -324,7 +324,7 @@ llc_print(netdissect_options *ndo, const u_char *p, u_int length, u_int caplen,
 #endif
        if (ssap == LLCSAP_ISONS && dsap == LLCSAP_ISONS
            && control == LLC_UI) {
-               isoclns_print(ndo, p, length, caplen);
+               isoclns_print(ndo, p, length);
                return (hdrlen);
        }
 

diff --git a/print-mpls.c b/print-mpls.c
index ba4223346c8bd20e331a7c0a3a99eca3af615e16..5c26e4f9b2a1a48c8d3555113766e5e165cddb4f 100644 (file)
--- a/print-mpls.c
+++ b/print-mpls.c
@@ -201,7 +201,7 @@ mpls_print(netdissect_options *ndo, const u_char *bp, u_int length)
                break;
 
        case PT_OSI:
-               isoclns_print(ndo, p, length, length);
+               isoclns_print(ndo, p, length);
                break;
 
        default:

diff --git a/print-null.c b/print-null.c
index 14df5fd15833e5371f143a31dd49fa1622bbc0cf..59b691cc63ba94bb76d36b7d578776095a3b58fc 100644 (file)
--- a/print-null.c
+++ b/print-null.c
@@ -117,7 +117,7 @@ null_if_print(netdissect_options *ndo, const struct pcap_pkthdr *h, const u_char
                break;
 
        case BSD_AFNUM_ISO:
-               isoclns_print(ndo, p, length, caplen);
+               isoclns_print(ndo, p, length);
                break;
 
        case BSD_AFNUM_APPLETALK:

diff --git a/print-ppp.c b/print-ppp.c
index b30f224c80529ba16d3aa1a12be4b767003eddae..134389209fcac0ac885aa8ee15a0670be2d4fd86 100644 (file)
--- a/print-ppp.c
+++ b/print-ppp.c
@@ -1484,7 +1484,7 @@ handle_ppp(netdissect_options *ndo,
                ipx_print(ndo, p, length);
                break;
        case PPP_OSI:
-               isoclns_print(ndo, p, length, length);
+               isoclns_print(ndo, p, length);
                break;
        case PPP_MPLS_UCAST:
        case PPP_MPLS_MCAST:

diff --git a/tests/TESTLIST b/tests/TESTLIST
index ee6fb46eac465fc763f951f61de3b3f23e8cafdc..986c18fe895cdca41fe293168996c078bd7e598b 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -442,6 +442,7 @@ stp-v4-length-sigsegv       stp-v4-length-sigsegv.pcap      stp-v4-length-sigsegv.out
 hoobr_pimv1            hoobr_pimv1.pcap                hoobr_pimv1.out
 hoobr_safeputs         hoobr_safeputs.pcap             hoobr_safeputs.out
 isakmp-rfc3948-oobr    isakmp-rfc3948-oobr.pcap        isakmp-rfc3948-oobr.out
+isoclns-oobr           isoclns-oobr.pcap               isoclns-oobr.out
 
 # bad packets from Wilfried Kirsch
 slip-bad-direction     slip-bad-direction.pcap         slip-bad-direction.out  -ve

diff --git a/tests/isoclns-oobr.out b/tests/isoclns-oobr.out
new file mode 100644 (file)
index 0000000..c2cfdfb
--- /dev/null
+++ b/tests/isoclns-oobr.out
@@ -0,0 +1 @@
+|OSI

diff --git a/tests/isoclns-oobr.pcap b/tests/isoclns-oobr.pcap
new file mode 100644 (file)
index 0000000..276b8a9
Binary files /dev/null and b/tests/isoclns-oobr.pcap differ