CVE-2017-13013/ARP: Fix printing of ARP protocol addresses.

If the protocol type isn't ETHERTYPE_IP or ETHERTYPE_TRAIL, or if the
protocol address length isn't 4, don't print the address as an IPv4 address.

This fixes a buffer over-read discovered by Bhargava Shastry,
SecT/TU Berlin.

Add a test using the capture file supplied by the reporter(s), modified
so the capture file won't be rejected as an invalid capture.

Update another test file's tcpdump output to reflect this change.

1 files changed, 72 insertions, 17 deletions

diff --git a/print-arp.c b/print-arp.c
index eff97c46..96727faa 100644
--- a/print-arp.c
+++ b/print-arp.c
@@ -78,7 +78,7 @@ struct  arp_pkthdr {
 	u_char	ar_tha[];	/* target hardware address */
 	u_char	ar_tpa[];	/* target protocol address */
 #endif
-#define ar_sha(ap)	(((const u_char *)((ap)+1))+0)
+#define ar_sha(ap)	(((const u_char *)((ap)+1))+  0)
 #define ar_spa(ap)	(((const u_char *)((ap)+1))+  (ap)->ar_hln)
 #define ar_tha(ap)	(((const u_char *)((ap)+1))+  (ap)->ar_hln+(ap)->ar_pln)
 #define ar_tpa(ap)	(((const u_char *)((ap)+1))+2*(ap)->ar_hln+(ap)->ar_pln)
@@ -190,6 +190,30 @@ isnonzero(const u_char *a, size_t len)
 }
 
 static void
+tpaddr_print_ip(netdissect_options *ndo,
+	        const struct arp_pkthdr *ap, u_short pro)
+{
+	if (pro != ETHERTYPE_IP && pro != ETHERTYPE_TRAIL)
+		ND_PRINT((ndo, "<wrong proto type>"));
+	else if (PROTO_LEN(ap) != 4)
+		ND_PRINT((ndo, "<wrong len>"));
+	else
+		ND_PRINT((ndo, "%s", ipaddr_string(ndo, TPA(ap))));
+}
+
+static void
+spaddr_print_ip(netdissect_options *ndo,
+	        const struct arp_pkthdr *ap, u_short pro)
+{
+	if (pro != ETHERTYPE_IP && pro != ETHERTYPE_TRAIL)
+		ND_PRINT((ndo, "<wrong proto type>"));
+	else if (PROTO_LEN(ap) != 4)
+		ND_PRINT((ndo, "<wrong len>"));
+	else
+		ND_PRINT((ndo, "%s", ipaddr_string(ndo, SPA(ap))));
+}
+
+static void
 atmarp_addr_print(netdissect_options *ndo,
 		  const u_char *ha, u_int ha_len, const u_char *srca,
     u_int srca_len)
@@ -205,6 +229,30 @@ atmarp_addr_print(netdissect_options *ndo,
 }
 
 static void
+atmarp_tpaddr_print(netdissect_options *ndo,
+		    const struct atmarp_pkthdr *ap, u_short pro)
+{
+	if (pro != ETHERTYPE_IP && pro != ETHERTYPE_TRAIL)
+		ND_PRINT((ndo, "<wrong proto type>"));
+	else if (ATMTPROTO_LEN(ap) != 4)
+		ND_PRINT((ndo, "<wrong tplen>"));
+	else
+		ND_PRINT((ndo, "%s", ipaddr_string(ndo, ATMTPA(ap))));
+}
+
+static void
+atmarp_spaddr_print(netdissect_options *ndo,
+		    const struct atmarp_pkthdr *ap, u_short pro)
+{
+	if (pro != ETHERTYPE_IP && pro != ETHERTYPE_TRAIL)
+		ND_PRINT((ndo, "<wrong proto type>"));
+	else if (ATMSPROTO_LEN(ap) != 4)
+		ND_PRINT((ndo, "<wrong splen>"));
+	else
+		ND_PRINT((ndo, "%s", ipaddr_string(ndo, ATMSPA(ap))));
+}
+
+static void
 atmarp_print(netdissect_options *ndo,
 	     const u_char *bp, u_int length, u_int caplen)
 {
@@ -252,18 +300,21 @@ atmarp_print(netdissect_options *ndo,
 	switch (op) {
 
 	case ARPOP_REQUEST:
-		ND_PRINT((ndo, "who-has %s", ipaddr_string(ndo, ATMTPA(ap))));
+		ND_PRINT((ndo, "who-has "));
+		atmarp_tpaddr_print(ndo, ap, pro);
 		if (ATMTHRD_LEN(ap) != 0) {
 			ND_PRINT((ndo, " ("));
 			atmarp_addr_print(ndo, ATMTHA(ap), ATMTHRD_LEN(ap),
 			    ATMTSA(ap), ATMTSLN(ap));
 			ND_PRINT((ndo, ")"));
 		}
-		ND_PRINT((ndo, "tell %s", ipaddr_string(ndo, ATMSPA(ap))));
+		ND_PRINT((ndo, " tell "));
+		atmarp_spaddr_print(ndo, ap, pro);
 		break;
 
 	case ARPOP_REPLY:
-		ND_PRINT((ndo, "%s is-at ", ipaddr_string(ndo, ATMSPA(ap))));
+		atmarp_spaddr_print(ndo, ap, pro);
+		ND_PRINT((ndo, " is-at "));
 		atmarp_addr_print(ndo, ATMSHA(ap), ATMSHRD_LEN(ap), ATMSSA(ap),
                                   ATMSSLN(ap));
 		break;
@@ -280,11 +331,13 @@ atmarp_print(netdissect_options *ndo,
 	case ARPOP_INVREPLY:
 		atmarp_addr_print(ndo, ATMSHA(ap), ATMSHRD_LEN(ap), ATMSSA(ap),
 		    ATMSSLN(ap));
-		ND_PRINT((ndo, "at %s", ipaddr_string(ndo, ATMSPA(ap))));
+		ND_PRINT((ndo, "at "));
+		atmarp_spaddr_print(ndo, ap, pro);
 		break;
 
 	case ARPOP_NAK:
-		ND_PRINT((ndo, "for %s", ipaddr_string(ndo, ATMSPA(ap))));
+		ND_PRINT((ndo, "for "));
+		atmarp_spaddr_print(ndo, ap, pro);
 		break;
 
 	default:
@@ -332,7 +385,7 @@ arp_print(netdissect_options *ndo,
             break;
 	}
 
-	if (!ND_TTEST2(*ar_tpa(ap), PROTO_LEN(ap))) {
+	if (!ND_TTEST2(*TPA(ap), PROTO_LEN(ap))) {
 		ND_PRINT((ndo, "%s", tstr));
 		ND_DEFAULTPRINT((const u_char *)ap, length);
 		return;
@@ -367,16 +420,18 @@ arp_print(netdissect_options *ndo,
 	switch (op) {
 
 	case ARPOP_REQUEST:
-		ND_PRINT((ndo, "who-has %s", ipaddr_string(ndo, TPA(ap))));
+		ND_PRINT((ndo, "who-has "));
+		tpaddr_print_ip(ndo, ap, pro);
 		if (isnonzero((const u_char *)THA(ap), HRD_LEN(ap)))
 			ND_PRINT((ndo, " (%s)",
 				  linkaddr_string(ndo, THA(ap), linkaddr, HRD_LEN(ap))));
-		ND_PRINT((ndo, " tell %s", ipaddr_string(ndo, SPA(ap))));
+		ND_PRINT((ndo, " tell "));
+		spaddr_print_ip(ndo, ap, pro);
 		break;
 
 	case ARPOP_REPLY:
-		ND_PRINT((ndo, "%s is-at %s",
-                          ipaddr_string(ndo, SPA(ap)),
+		spaddr_print_ip(ndo, ap, pro);
+		ND_PRINT((ndo, " is-at %s",
                           linkaddr_string(ndo, SHA(ap), linkaddr, HRD_LEN(ap))));
 		break;
 
@@ -387,9 +442,9 @@ arp_print(netdissect_options *ndo,
 		break;
 
 	case ARPOP_REVREPLY:
-		ND_PRINT((ndo, "%s at %s",
-			  linkaddr_string(ndo, THA(ap), linkaddr, HRD_LEN(ap)),
-			  ipaddr_string(ndo, TPA(ap))));
+		ND_PRINT((ndo, "%s at ",
+			  linkaddr_string(ndo, THA(ap), linkaddr, HRD_LEN(ap))));
+		tpaddr_print_ip(ndo, ap, pro);
 		break;
 
 	case ARPOP_INVREQUEST:
@@ -399,9 +454,9 @@ arp_print(netdissect_options *ndo,
 		break;
 
 	case ARPOP_INVREPLY:
-		ND_PRINT((ndo,"%s at %s",
-			  linkaddr_string(ndo, SHA(ap), linkaddr, HRD_LEN(ap)),
-			  ipaddr_string(ndo, SPA(ap))));
+		ND_PRINT((ndo,"%s at ",
+			  linkaddr_string(ndo, SHA(ap), linkaddr, HRD_LEN(ap))));
+		spaddr_print_ip(ndo, ap, pro);
 		break;
 
 	default: