Wireshark · Ethereal-dev: Re: [ethereal-dev] Short packets / Bad data

[Gilbert Ramirez <gram@xxxxxxxxxx>]

On Sat, Dec 04, 1999 at 08:28:47PM -0600, Nathan Neulinger wrote:
> 
> In a few of the dissectors I have written, I have defined a macro like:
> 
> #define TRUNC(x) { if(!BYTES_ARE_IN_FRAME(offset,(x)) { \
> 	dissect_data(pd,offset,fd,tree); \
> 	return; }
> 
> That could be extended a bit to put some text on the proto tree saying
> it was truncated or whatever.
> 
> I then just prefix any of my dissections with calls to TRUNC(). It's not
> the most efficient method in the world, but it makes coding really easy.

I was starting to experiment with a setjmp()/longjmp() system, where
dissect_packet() would call setjmp(), and all data accesses into pd[]
were done via macros. In fact, I'd probably move pd into the packet_info
struct, and keep it out of the argument stack. If code tries to retrieve
data from beyond the bounds of pd[], longjmp() is called and we return
safely back to dissect_packet().

What I didn't like about my code is that if a dissector does a bunch
of data retrievals at its beginning, and then later puts all this data
into the proto_tree, then it may longjmp() before it puts *anything*
into the proto_tree. This is not good; I was striving to put as much
as possible into the proto_tree. 

Anyway, here's the diff of what I was playing with, and a sample
very-short-packet ethernet trace made with randpkt, in case anyone
is interested.

--gilbert

Index: packet-eth.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-eth.c,v
retrieving revision 1.25
diff -u -r1.25 packet-eth.c
--- packet-eth.c	1999/11/30 23:56:35	1.25
+++ packet-eth.c	1999/12/05 04:41:05
@@ -111,25 +111,24 @@
 
 void
 dissect_eth(const u_char *pd, int offset, frame_data *fd, proto_tree *tree) {
-  guint16    etype, length;
-  proto_tree *fh_tree = NULL;
-  proto_item *ti;
-  int        ethhdr_type;	/* the type of ethernet frame */
-  
-  if (fd->cap_len < ETH_HEADER_SIZE) {
-    dissect_data(pd, offset, fd, tree);
-    return;
-  }
-
-  SET_ADDRESS(&pi.dl_src, AT_ETHER, 6, &pd[offset+6]);
-  SET_ADDRESS(&pi.src, AT_ETHER, 6, &pd[offset+6]);
-  SET_ADDRESS(&pi.dl_dst, AT_ETHER, 6, &pd[offset+0]);
-  SET_ADDRESS(&pi.dst, AT_ETHER, 6, &pd[offset+0]);
+  guint16	etype, length, ipx_check;
+  proto_tree	*fh_tree = NULL;
+  proto_item	*ti;
+  int		ethhdr_type;	/* the type of ethernet frame */
+  const guint8	*src_mac, *dst_mac;
+
+  dst_mac = FRAME_DATA_PTR(offset,   6);
+  src_mac = FRAME_DATA_PTR(offset+6, 6);
+
+  SET_ADDRESS(&pi.dl_src, AT_ETHER, 6, src_mac);
+  SET_ADDRESS(&pi.src,    AT_ETHER, 6, src_mac);
+  SET_ADDRESS(&pi.dl_dst, AT_ETHER, 6, dst_mac);
+  SET_ADDRESS(&pi.dst,    AT_ETHER, 6, dst_mac);
 
   if (check_col(fd, COL_PROTOCOL))
     col_add_str(fd, COL_PROTOCOL, "Ethernet");
 
-  etype = pntohs(&pd[offset+12]);
+  etype = FRAME_DATA_NTOHS(offset+12);
 
 	/* either ethernet802.3 or ethernet802.2 */
   if (etype <= IEEE_802_3_MAX_LEN) {
@@ -141,7 +140,9 @@
        (IPX/SPX is they only thing that can be contained inside a
        straight 802.3 packet). A non-0xffff value means that there's an
        802.2 layer inside the 802.3 layer */
-    if (pd[offset+14] == 0xff && pd[offset+15] == 0xff) {
+
+    ipx_check = FRAME_DATA_NTOHS(offset+14);
+    if (ipx_check == 0xffff) {
       ethhdr_type = ETHERNET_802_3;
     }
     else {
@@ -159,8 +160,8 @@
 
 	fh_tree = proto_item_add_subtree(ti, ett_ieee8023);
 
-	proto_tree_add_item(fh_tree, hf_eth_dst, offset+0, 6, &pd[offset+0]);
-	proto_tree_add_item(fh_tree, hf_eth_src, offset+6, 6, &pd[offset+6]);
+	proto_tree_add_item(fh_tree, hf_eth_dst, offset+0, 6, dst_mac);
+	proto_tree_add_item(fh_tree, hf_eth_src, offset+6, 6, src_mac);
 	proto_tree_add_item(fh_tree, hf_eth_len, offset+12, 2, length);
 
 	/* Convert the LLC length from the 802.3 header to a total
@@ -185,8 +186,8 @@
 
 	fh_tree = proto_item_add_subtree(ti, ett_ether2);
 
-	proto_tree_add_item(fh_tree, hf_eth_dst, offset+0, 6, &pd[offset+0]);
-	proto_tree_add_item(fh_tree, hf_eth_src, offset+6, 6, &pd[offset+6]);
+	proto_tree_add_item(fh_tree, hf_eth_dst, offset+0, 6, dst_mac);
+	proto_tree_add_item(fh_tree, hf_eth_src, offset+6, 6, src_mac);
     }
   }
   offset += ETH_HEADER_SIZE;
Index: packet.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet.c,v
retrieving revision 1.58
diff -u -r1.58 packet.c
--- packet.c	1999/12/02 01:33:55	1.58
+++ packet.c	1999/12/05 04:41:08
@@ -763,13 +763,21 @@
 	g_slist_foreach(init_routines, &call_init_routine, NULL);
 }
 
+
+int
+longjmp_retval(jmp_buf env, int retval)
+{
+	longjmp(env, retval);
+	return 1;
+}
+
 /* this routine checks the frame type from the cf structure */
 void
 dissect_packet(const u_char *pd, frame_data *fd, proto_tree *tree)
 {
-	proto_tree *fh_tree;
-	proto_item *ti;
-	struct timeval tv;
+	proto_tree	*fh_tree;
+	proto_item	*ti;
+	struct timeval	tv;
 
 	/* Put in frame header information. */
 	if (tree) {
@@ -810,43 +818,51 @@
 	pi.len = fd->pkt_len;
 	pi.captured_len = fd->cap_len;
 
-	switch (fd->lnk_t) {
-		case WTAP_ENCAP_ETHERNET :
-			dissect_eth(pd, 0, fd, tree);
-			break;
-		case WTAP_ENCAP_FDDI :
-			dissect_fddi(pd, fd, tree, FALSE);
-			break;
-		case WTAP_ENCAP_FDDI_BITSWAPPED :
-			dissect_fddi(pd, fd, tree, TRUE);
-			break;
-		case WTAP_ENCAP_TR :
-			dissect_tr(pd, 0, fd, tree);
-			break;
-		case WTAP_ENCAP_NULL :
-			dissect_null(pd, fd, tree);
-			break;
-		case WTAP_ENCAP_PPP :
-			dissect_ppp(pd, fd, tree);
-			break;
-		case WTAP_ENCAP_LAPB :
-			dissect_lapb(pd, fd, tree);
-			break;
-		case WTAP_ENCAP_RAW_IP :
-			dissect_raw(pd, fd, tree);
-			break;
-		case WTAP_ENCAP_LINUX_ATM_CLIP :
-			dissect_clip(pd, fd, tree);
-			break;
-		case WTAP_ENCAP_ATM_SNIFFER :
-			dissect_atm(pd, fd, tree);
-			break;
-		case WTAP_ENCAP_ASCEND :
-			dissect_ascend(pd, fd, tree);
-			break;
-		case WTAP_ENCAP_LAPD :
-			dissect_lapd(pd, fd, tree);
-			break;
+	if (setjmp(pi.jmp_env) == 0) {
+		switch (fd->lnk_t) {
+			case WTAP_ENCAP_ETHERNET :
+				dissect_eth(pd, 0, fd, tree);
+				break;
+			case WTAP_ENCAP_FDDI :
+				dissect_fddi(pd, fd, tree, FALSE);
+				break;
+			case WTAP_ENCAP_FDDI_BITSWAPPED :
+				dissect_fddi(pd, fd, tree, TRUE);
+				break;
+			case WTAP_ENCAP_TR :
+				dissect_tr(pd, 0, fd, tree);
+				break;
+			case WTAP_ENCAP_NULL :
+				dissect_null(pd, fd, tree);
+				break;
+			case WTAP_ENCAP_PPP :
+				dissect_ppp(pd, fd, tree);
+				break;
+			case WTAP_ENCAP_LAPB :
+				dissect_lapb(pd, fd, tree);
+				break;
+			case WTAP_ENCAP_RAW_IP :
+				dissect_raw(pd, fd, tree);
+				break;
+			case WTAP_ENCAP_LINUX_ATM_CLIP :
+				dissect_clip(pd, fd, tree);
+				break;
+			case WTAP_ENCAP_ATM_SNIFFER :
+				dissect_atm(pd, fd, tree);
+				break;
+			case WTAP_ENCAP_ASCEND :
+				dissect_ascend(pd, fd, tree);
+				break;
+			case WTAP_ENCAP_LAPD :
+				dissect_lapd(pd, fd, tree);
+				break;
+		}
+	}
+	else {
+		/* we returned from a longjmp */
+/*		if (tree)  {
+			proto_tree_add_text(fh_tree, 0, 0, "[Unexpected end of frame]");
+		}*/
 	}
 }
 
Index: packet.h
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet.h,v
retrieving revision 1.155
diff -u -r1.155 packet.h
--- packet.h	1999/12/03 21:50:31	1.155
+++ packet.h	1999/12/05 04:41:11
@@ -27,6 +27,8 @@
 #ifndef __PACKET_H__
 #define __PACKET_H__
 
+#include <setjmp.h>
+
 #ifndef __WTAP_H__
 #include "wiretap/wtap.h"
 #endif
@@ -79,10 +81,27 @@
 
 /* Check whether there's any data at all starting at "offset". */
 #define	IS_DATA_IN_FRAME(offset)	((offset) < pi.captured_len)
-		
+
+/* Return pointer to frame data, checking frame length */
+#define FRAME_DATA_PTR(offset, len) \
+				((offset) + (len) <= pi.captured_len) ? \
+					&pd[(offset)] : \
+					(guint8*)longjmp_retval(pi.jmp_env, 1)
+
+/* Return host short converted from network short, checking frame length */
+#define FRAME_DATA_NTOHS(offset) \
+				   ((offset) + 2 <= pi.captured_len) ? \
+					pntohs(&pd[(offset)]) : \
+					longjmp_retval(pi.jmp_env, 1)
+
+/* Calls longjmp() and pretends to return a value, to placate
+ * the compiler when I use longjmp() as part of a "?:" expression. */
+int longjmp_retval(jmp_buf, int);
+
 /* To pass one of two strings, singular or plural */
 #define plurality(d,s,p) ((d) == 1 ? (s) : (p))
 
+
 typedef struct _column_info {
   gint       num_cols;  /* Number of columns */
   gint      *col_fmt;   /* Format of column */
@@ -187,6 +206,7 @@
   guint32 match_port;
   int     iplen;
   int     iphdrlen;
+  jmp_buf jmp_env;
 } packet_info;
 
 extern packet_info pi;

Attachment: short-eth.cap
Description: Binary data