Skip to content

Commit

Permalink
CVE-2017-12897/ISO CLNS: Use ND_TTEST() for the bounds checks in isoc…
Browse files Browse the repository at this point in the history 
…lns_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).
  • Loading branch information
@guyharris @infrastation
guyharris authored and infrastation committed Sep 13, 2017
1 parent f76e7fe commit 1dcd10a
Show file tree
Hide file tree
Showing 15 changed files with 27 additions and 26 deletions.
2 changes: 1 addition & 1 deletion netdissect.h
View file
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion print-atm.c
View file
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions print-chdlc.c
View file
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion print-ether.c
View file
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion print-fr.c
View file
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion print-gre.c
View file
Expand Up @@ -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);
Expand Down
13 changes: 6 additions & 7 deletions print-isoclns.c
View file
Expand Up @@ -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;
}
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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;
}
}
Expand Down
16 changes: 8 additions & 8 deletions print-juniper.c
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion print-llc.c
View file
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion print-mpls.c
View file
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion print-null.c
View file
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion print-ppp.c
View file
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions tests/TESTLIST
View file
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions tests/isoclns-oobr.out
View file
@@ -0,0 +1 @@
|OSI
Binary file added tests/isoclns-oobr.pcap
View file
Binary file not shown.

0 comments on commit 1dcd10a

Please sign in to comment.