AppleTalk: Modernize packet parsing style.

Enable ND_LONGJMP_FROM_TCHECK. Report invalid packets as invalid. Remove
all improvised snapshot end guards as they were redundant. In
print_cstring() have nd_printjn() guard the snapshot end. Use tok2str()
in nbp_print(). Update two tests.

4 files changed, 69 insertions, 108 deletions

diff --git a/CHANGES b/CHANGES
index 20959f36..b5611c80 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,7 +2,7 @@ Monthday, Month DD, YYYY by gharris and denis
   Summary for 5.0.0 tcpdump release (so far!)
     Source code:
       Use %zu when printing a sizeof to squelch compiler warnings
-      BOOTP, EGP, EIGRP, Geneve, L2TP, OLSR, PGM, RSVP, UDP: Modernize packet parsing style
+      AppleTalk, BOOTP, EGP, EIGRP, Geneve, L2TP, OLSR, PGM, RSVP, UDP: Modernize packet parsing style
       EGP: Replace custom code with tok2str()
       EIGRP: Get the packet header fields right.
       UDP: Clean up address and port printing.
diff --git a/print-atalk.c b/print-atalk.c
index ae191397..9cca576e 100644
--- a/print-atalk.c
+++ b/print-atalk.c
@@ -30,6 +30,7 @@
 #include <stdio.h>
 #include <string.h>
 
+#define ND_LONGJMP_FROM_TCHECK
 #include "netdissect.h"
 #include "addrtoname.h"
 #include "ethertype.h"
@@ -94,6 +95,12 @@ struct atNBPtuple {
 #define	nbpBrRq		0x10
 #define	nbpLkUp		0x20
 #define	nbpLkUpReply	0x30
+static const struct tok nbp_str[] = {
+	{ nbpBrRq,      "brRq"  },
+	{ nbpLkUp,      "lkup"  },
+	{ nbpLkUpReply, "reply" },
+	{ 0, NULL }
+};
 
 #define	ddpRTMP		1	/* RTMP type */
 #define	ddpNBP		2	/* NBP type */
@@ -128,10 +135,8 @@ static void atp_print(netdissect_options *, const struct atATP *, u_int);
 static void atp_bitmap_print(netdissect_options *, u_char);
 static void nbp_print(netdissect_options *, const struct atNBP *, u_int, u_short, u_char, u_char);
 static const struct atNBPtuple *nbp_tuple_print(netdissect_options *ndo, const struct atNBPtuple *,
-						const u_char *,
 						u_short, u_char, u_char);
-static const struct atNBPtuple *nbp_name_print(netdissect_options *, const struct atNBPtuple *,
-					       const u_char *);
+static const struct atNBPtuple *nbp_name_print(netdissect_options *, const struct atNBPtuple *);
 static const char *ataddr_string(netdissect_options *, u_short, u_char);
 static void ddp_print(netdissect_options *, const u_char *, u_int, u_int, u_short, u_char, u_char);
 static const char *ddpskt_string(netdissect_options *, u_int);
@@ -143,16 +148,8 @@ void
 ltalk_if_print(netdissect_options *ndo,
                const struct pcap_pkthdr *h, const u_char *p)
 {
-	u_int hdrlen;
-
 	ndo->ndo_protocol = "ltalk";
-	hdrlen = llap_print(ndo, p, h->len);
-	if (hdrlen == 0) {
-		/* Cut short by the snapshot length. */
-		ndo->ndo_ll_hdr_len += h->caplen;
-		return;
-	}
-	ndo->ndo_ll_hdr_len += hdrlen;
+	ndo->ndo_ll_hdr_len += llap_print(ndo, p, h->len);
 }
 
 /*
@@ -170,12 +167,8 @@ llap_print(netdissect_options *ndo,
 
 	ndo->ndo_protocol = "llap";
 	if (length < sizeof(*lp)) {
-		ND_PRINT(" [|llap %u]", length);
-		return (length);
-	}
-	if (!ND_TTEST_LEN(bp, sizeof(*lp))) {
-		nd_print_trunc(ndo);
-		return (0);	/* cut short by the snapshot length */
+		ND_PRINT(" (LLAP length %u is too small)", length);
+		goto invalid;
 	}
 	lp = (const struct LAP *)bp;
 	bp += sizeof(*lp);
@@ -184,13 +177,10 @@ llap_print(netdissect_options *ndo,
 	switch (GET_U_1(lp->type)) {
 
 	case lapShortDDP:
+		ndo->ndo_protocol = "sddp";
 		if (length < ddpSSize) {
-			ND_PRINT(" [|sddp %u]", length);
-			return (length);
-		}
-		if (!ND_TTEST_LEN(bp, ddpSSize)) {
-			ND_PRINT(" [|sddp]");
-			return (0);	/* cut short by the snapshot length */
+			ND_PRINT(" (SDDP length %u is too small)", length);
+			goto invalid;
 		}
 		sdp = (const struct atShortDDP *)bp;
 		ND_PRINT("%s.%s",
@@ -207,13 +197,10 @@ llap_print(netdissect_options *ndo,
 		break;
 
 	case lapDDP:
+		ndo->ndo_protocol = "ddp";
 		if (length < ddpSize) {
-			ND_PRINT(" [|ddp %u]", length);
-			return (length);
-		}
-		if (!ND_TTEST_LEN(bp, ddpSize)) {
-			ND_PRINT(" [|ddp]");
-			return (0);	/* cut short by the snapshot length */
+			ND_PRINT(" (DDP length %u is too small)", length);
+			goto invalid;
 		}
 		dp = (const struct atDDP *)bp;
 		snet = GET_BE_U_2(dp->srcNet);
@@ -237,6 +224,9 @@ llap_print(netdissect_options *ndo,
 		break;
 	}
 	return (hdrlen);
+invalid:
+	nd_print_invalid(ndo);
+	return length;
 }
 
 /*
@@ -256,12 +246,8 @@ atalk_print(netdissect_options *ndo,
             ND_PRINT("AT ");
 
 	if (length < ddpSize) {
-		ND_PRINT(" [|ddp %u]", length);
-		return;
-	}
-	if (!ND_TTEST_LEN(bp, ddpSize)) {
-		ND_PRINT(" [|ddp]");
-		return;
+		ND_PRINT(" (length %u is too small)", length);
+		goto invalid;
 	}
 	dp = (const struct atDDP *)bp;
 	snet = GET_BE_U_2(dp->srcNet);
@@ -274,6 +260,9 @@ atalk_print(netdissect_options *ndo,
 	length -= ddpSize;
 	ddp_print(ndo, bp, length, GET_U_1(dp->type), snet,
 		  GET_U_1(dp->srcNode), GET_U_1(dp->srcSkt));
+	return;
+invalid:
+	nd_print_invalid(ndo);
 }
 
 /* XXX should probably pass in the snap header and do checks like arp_print() */
@@ -283,20 +272,18 @@ aarp_print(netdissect_options *ndo,
 {
 	const struct aarp *ap;
 
-#define AT(member) ataddr_string(ndo, (ap->member[1]<<8)|ap->member[2],ap->member[3])
+#define AT(member) ataddr_string(ndo, \
+	(GET_U_1(&ap->member[1])<<8)|GET_U_1(&ap->member[2]), \
+	GET_U_1(&ap->member[3]))
 
 	ndo->ndo_protocol = "aarp";
 	ND_PRINT("aarp ");
 	ap = (const struct aarp *)bp;
-	if (!ND_TTEST_SIZE(ap)) {
-		/* Just bail if we don't have the whole chunk. */
-		nd_print_trunc(ndo);
-		return;
-	}
 	if (length < sizeof(*ap)) {
-		ND_PRINT(" [|aarp %u]", length);
-		return;
+		ND_PRINT(" (length %u is too small)", length);
+		goto invalid;
 	}
+	ND_TCHECK_SIZE(ap);
 	if (GET_BE_U_2(ap->htype) == 1 &&
 	    GET_BE_U_2(ap->ptype) == ETHERTYPE_ATALK &&
 	    GET_U_1(ap->halen) == MAC_ADDR_LEN && GET_U_1(ap->palen) == 4)
@@ -317,6 +304,9 @@ aarp_print(netdissect_options *ndo,
 	ND_PRINT("len %u op %u htype %u ptype %#x halen %u palen %u",
 	    length, GET_BE_U_2(ap->op), GET_BE_U_2(ap->htype),
 	    GET_BE_U_2(ap->ptype), GET_U_1(ap->halen), GET_U_1(ap->palen));
+	return;
+invalid:
+	nd_print_invalid(ndo);
 }
 
 /*
@@ -355,14 +345,10 @@ atp_print(netdissect_options *ndo,
 	uint8_t control;
 	uint32_t data;
 
-	if ((const u_char *)(ap + 1) > ndo->ndo_snapend) {
-		/* Just bail if we don't have the whole chunk. */
-		nd_print_trunc(ndo);
-		return;
-	}
+	ndo->ndo_protocol = "atp";
 	if (length < sizeof(*ap)) {
-		ND_PRINT(" [|atp %u]", length);
-		return;
+		ND_PRINT(" (ATP length %u is too small)", length);
+		goto invalid;
 	}
 	length -= sizeof(*ap);
 	control = GET_U_1(ap->control);
@@ -444,6 +430,9 @@ atp_print(netdissect_options *ndo,
 	data = GET_BE_U_4(ap->userData);
 	if (data != 0)
 		ND_PRINT(" 0x%x", data);
+	return;
+invalid:
+	nd_print_invalid(ndo);
 }
 
 static void
@@ -486,37 +475,22 @@ nbp_print(netdissect_options *ndo,
 		(const struct atNBPtuple *)((const u_char *)np + nbpHeaderSize);
 	uint8_t control;
 	u_int i;
-	const u_char *ep;
-
-	if (length < nbpHeaderSize) {
-		ND_PRINT(" truncated-nbp %u", length);
-		return;
-	}
 
-	length -= nbpHeaderSize;
-	if (length < 8) {
+	if (length < nbpHeaderSize + 8) {
 		/* must be room for at least one tuple */
-		ND_PRINT(" truncated-nbp %u", length + nbpHeaderSize);
-		return;
-	}
-	/* ep points to end of available data */
-	ep = ndo->ndo_snapend;
-	if ((const u_char *)tp > ep) {
-		nd_print_trunc(ndo);
-		return;
+		ND_PRINT(" undersized-nbp %u", length);
+		goto invalid;
 	}
+	length -= nbpHeaderSize;
 	control = GET_U_1(np->control);
-	switch (i = (control & 0xf0)) {
+	ND_PRINT(" nbp-%s", tok2str(nbp_str, "0x%x", control & 0xf0));
+	ND_PRINT(" %u", GET_U_1(np->id));
+	switch (control & 0xf0) {
 
 	case nbpBrRq:
 	case nbpLkUp:
-		ND_PRINT(i == nbpLkUp? " nbp-lkup %u:":" nbp-brRq %u:",
-			 GET_U_1(np->id));
-		if ((const u_char *)(tp + 1) > ep) {
-			nd_print_trunc(ndo);
-			return;
-		}
-		(void)nbp_name_print(ndo, tp, ep);
+		ND_PRINT(":");
+		(void)nbp_name_print(ndo, tp);
 		/*
 		 * look for anomalies: the spec says there can only
 		 * be one tuple, the address must match the source
@@ -536,63 +510,50 @@ nbp_print(netdissect_options *ndo,
 		break;
 
 	case nbpLkUpReply:
-		ND_PRINT(" nbp-reply %u:", GET_U_1(np->id));
+		ND_PRINT(":");
 
 		/* print each of the tuples in the reply */
 		for (i = control & 0xf; i != 0 && tp; i--)
-			tp = nbp_tuple_print(ndo, tp, ep, snet, snode, skt);
+			tp = nbp_tuple_print(ndo, tp, snet, snode, skt);
 		break;
 
 	default:
-		ND_PRINT(" nbp-0x%x  %u (%u)", control, GET_U_1(np->id),
-			 length);
+		ND_PRINT("  (%u)", length);
 		break;
 	}
+	return;
+invalid:
+	nd_print_invalid(ndo);
 }
 
 /* print a counted string */
 static const u_char *
 print_cstring(netdissect_options *ndo,
-              const u_char *cp, const u_char *ep)
+              const u_char *cp)
 {
 	u_int length;
 
-	if (cp >= ep) {
-		nd_print_trunc(ndo);
-		return (0);
-	}
 	length = GET_U_1(cp);
 	cp++;
 
 	/* Spec says string can be at most 32 bytes long */
 	if (length > 32) {
 		ND_PRINT("[len=%u]", length);
-		return (0);
+		ND_TCHECK_LEN(cp, length);
+		return NULL;
 	}
-	while (length != 0) {
-		if (cp >= ep) {
-			nd_print_trunc(ndo);
-			return (0);
-		}
-		fn_print_char(ndo, GET_U_1(cp));
-		cp++;
-		length--;
-	}
-	return (cp);
+	nd_printjn(ndo, cp, length);
+	return cp + length;
 }
 
 static const struct atNBPtuple *
 nbp_tuple_print(netdissect_options *ndo,
-                const struct atNBPtuple *tp, const u_char *ep,
+                const struct atNBPtuple *tp,
                 u_short snet, u_char snode, u_char skt)
 {
 	const struct atNBPtuple *tpn;
 
-	if ((const u_char *)(tp + 1) > ep) {
-		nd_print_trunc(ndo);
-		return 0;
-	}
-	tpn = nbp_name_print(ndo, tp, ep);
+	tpn = nbp_name_print(ndo, tp);
 
 	/* if the enumerator isn't 1, print it */
 	if (GET_U_1(tp->enumerator) != 1)
@@ -613,7 +574,7 @@ nbp_tuple_print(netdissect_options *ndo,
 
 static const struct atNBPtuple *
 nbp_name_print(netdissect_options *ndo,
-               const struct atNBPtuple *tp, const u_char *ep)
+               const struct atNBPtuple *tp)
 {
 	const u_char *cp = (const u_char *)tp + nbpTupleSize;
 
@@ -621,13 +582,13 @@ nbp_name_print(netdissect_options *ndo,
 
 	/* Object */
 	ND_PRINT("\"");
-	if ((cp = print_cstring(ndo, cp, ep)) != NULL) {
+	if ((cp = print_cstring(ndo, cp)) != NULL) {
 		/* Type */
 		ND_PRINT(":");
-		if ((cp = print_cstring(ndo, cp, ep)) != NULL) {
+		if ((cp = print_cstring(ndo, cp)) != NULL) {
 			/* Zone */
 			ND_PRINT("@");
-			if ((cp = print_cstring(ndo, cp, ep)) != NULL)
+			if ((cp = print_cstring(ndo, cp)) != NULL)
 				ND_PRINT("\"");
 		}
 	}
diff --git a/tests/heapoverflow-EXTRACT_16BITS.out b/tests/heapoverflow-EXTRACT_16BITS.out
index 33e973b5..982db42a 100644
--- a/tests/heapoverflow-EXTRACT_16BITS.out
+++ b/tests/heapoverflow-EXTRACT_16BITS.out
@@ -1 +1 @@
-    1  05:27:12.808464432 et1 AT  [|ddp]
+    1  05:27:12.808464432 et1 AT  [|atalk]
diff --git a/tests/heapoverflow-atalk_print.out b/tests/heapoverflow-atalk_print.out
index 33e973b5..982db42a 100644
--- a/tests/heapoverflow-atalk_print.out
+++ b/tests/heapoverflow-atalk_print.out
@@ -1 +1 @@
-    1  05:27:12.808464432 et1 AT  [|ddp]
+    1  05:27:12.808464432 et1 AT  [|atalk]