From 13ab8d18617d616c7d343530f8a842e7143fb5cc Mon Sep 17 00:00:00 2001
From: Guy Harris <guy@alum.mit.edu>
Date: Thu, 16 Mar 2017 10:54:31 -0700
Subject: [PATCH] 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.
---
 print-arp.c                     |  89 +++++++++++---
 tests/TESTLIST                  |   1 +
 tests/arp-oobr.out              | 198 ++++++++++++++++----------------
 tests/getname_2_read4_asan.out  |   1 +
 tests/getname_2_read4_asan.pcap | Bin 0 -> 62 bytes
 5 files changed, 173 insertions(+), 116 deletions(-)
 create mode 100644 tests/getname_2_read4_asan.out
 create mode 100644 tests/getname_2_read4_asan.pcap

Index: tcpdump-3.9.8/print-arp.c
===================================================================
--- tcpdump-3.9.8.orig/print-arp.c
+++ tcpdump-3.9.8/print-arp.c
@@ -77,7 +77,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)
@@ -168,6 +168,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 (PLN(ap) != 4)
+		ND_PRINT((ndo, "<wrong len>"));
+	else
+		ND_PRINT((ndo, "%s", ipaddr_string(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 (PLN(ap) != 4)
+		ND_PRINT((ndo, "<wrong len>"));
+	else
+		ND_PRINT((ndo, "%s", ipaddr_string(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)
@@ -183,6 +207,30 @@ atmarp_addr_print(netdissect_options *nd
 }
 
 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 (ATMTPLN(ap) != 4)
+		ND_PRINT((ndo, "<wrong tplen>"));
+	else
+		ND_PRINT((ndo, "%s", ipaddr_string(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 (ATMSPLN(ap) != 4)
+		ND_PRINT((ndo, "<wrong splen>"));
+	else
+		ND_PRINT((ndo, "%s", ipaddr_string(ATMSPA(ap))));
+}
+
+static void
 atmarp_print(netdissect_options *ndo,
 	     const u_char *bp, u_int length, u_int caplen)
 {
@@ -213,18 +261,20 @@ atmarp_print(netdissect_options *ndo,
 	switch (op) {
 
 	case ARPOP_REQUEST:
-		ND_PRINT((ndo, "arp who-has %s", ipaddr_string(ATMTPA(ap))));
+		ND_PRINT((ndo, "who-has "));
+		atmarp_tpaddr_print(ndo, ap, pro);
 		if (ATMTHLN(ap) != 0) {
 			ND_PRINT((ndo, " ("));
 			atmarp_addr_print(ndo, ATMTHA(ap), ATMTHLN(ap),
 			    ATMTSA(ap), ATMTSLN(ap));
 			ND_PRINT((ndo, ")"));
 		}
-		ND_PRINT((ndo, " tell %s", ipaddr_string(ATMSPA(ap))));
+		ND_PRINT((ndo, " tell "));
+		atmarp_spaddr_print(ndo, ap, pro);
 		break;
 
 	case ARPOP_REPLY:
-		ND_PRINT((ndo, "arp reply %s", ipaddr_string(ATMSPA(ap))));
+		atmarp_spaddr_print(ndo, ap, pro);
 		ND_PRINT((ndo, " is-at "));
 		atmarp_addr_print(ndo, ATMSHA(ap), ATMSHLN(ap), ATMSSA(ap),
 		    ATMSSLN(ap));
@@ -240,15 +290,15 @@ atmarp_print(netdissect_options *ndo,
 		break;
 
 	case ARPOP_INVREPLY:
-		ND_PRINT((ndo, "invarp reply "));
 		atmarp_addr_print(ndo, ATMSHA(ap), ATMSHLN(ap), ATMSSA(ap),
 		    ATMSSLN(ap));
-		ND_PRINT((ndo, " at %s", ipaddr_string(ATMSPA(ap))));
+		ND_PRINT((ndo, "at "));
+		atmarp_spaddr_print(ndo, ap, pro);
 		break;
 
 	case ATMARPOP_NAK:
-		ND_PRINT((ndo, "nak reply for %s",
-			  ipaddr_string(ATMSPA(ap))));
+		ND_PRINT((ndo, "for "));
+		atmarp_spaddr_print(ndo, ap, pro);
 		break;
 
 	default:
@@ -278,7 +328,7 @@ arp_print(netdissect_options *ndo,
 	pro = PRO(ap);
 	op = OP(ap);
 
-	if (!ND_TTEST2(*ar_tpa(ap), PLN(ap))) {
+	if (!ND_TTEST2(*TPA(ap), PLN(ap))) {
 		ND_PRINT((ndo, "truncated-arp"));
 		ND_DEFAULTPRINT((const u_char *)ap, length);
 		return;
@@ -295,16 +345,19 @@ arp_print(netdissect_options *ndo,
 	switch (op) {
 
 	case ARPOP_REQUEST:
-		ND_PRINT((ndo, "arp who-has %s", ipaddr_string(TPA(ap))));
+		ND_PRINT((ndo, "who-has "));
+		tpaddr_print_ip(ndo, ap, pro);
 		if (isnonzero((const u_char *)THA(ap), HLN(ap)))
 			ND_PRINT((ndo, " (%s)",
 				  linkaddr_string(THA(ap), HLN(ap))));
-		ND_PRINT((ndo, " tell %s", ipaddr_string(SPA(ap))));
+		ND_PRINT((ndo, " tell "));
+		spaddr_print_ip(ndo, ap, pro);
 		break;
 
 	case ARPOP_REPLY:
-		ND_PRINT((ndo, "arp reply %s", ipaddr_string(SPA(ap))));
-		ND_PRINT((ndo, " is-at %s", linkaddr_string(SHA(ap), HLN(ap))));
+		spaddr_print_ip(ndo, ap, pro);
+		ND_PRINT((ndo, " is-at %s",
+                          linkaddr_string(SHA(ap), HLN(ap))));
 		break;
 
 	case ARPOP_REVREQUEST:
@@ -314,9 +367,9 @@ arp_print(netdissect_options *ndo,
 		break;
 
 	case ARPOP_REVREPLY:
-		ND_PRINT((ndo, "rarp reply %s at %s",
-			  linkaddr_string(THA(ap), HLN(ap)),
-			  ipaddr_string(TPA(ap))));
+		ND_PRINT((ndo, "%s at ",
+			  linkaddr_string(THA(ap), HLN(ap))));
+		tpaddr_print_ip(ndo, ap, pro);
 		break;
 
 	case ARPOP_INVREQUEST:
@@ -326,9 +379,9 @@ arp_print(netdissect_options *ndo,
 		break;
 
 	case ARPOP_INVREPLY:
-		ND_PRINT((ndo,"invarp reply %s at %s",
-			  linkaddr_string(THA(ap), HLN(ap)),
-			  ipaddr_string(TPA(ap))));
+		ND_PRINT((ndo,"%s at ",
+			  linkaddr_string(SHA(ap), HLN(ap))));
+		spaddr_print_ip(ndo, ap, pro);
 		break;
 
 	default:
