Ethereal-dev: Re: [Ethereal-dev] Ethereal crashes while thereal works for my decoder

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Guy Harris <guy@xxxxxxxxxx>
Date: Fri, 19 Jan 2001 15:30:36 -0800 (PST)
> I just started finding my way to write decoders, what I don't understand
> is, when I finish load my amf data, the GUI window crashes. If I use
> thereal -r, it works fine. Attached my core dump, and Here's my
> dissector:
> 
> dissect_amf(const u_char *pd, int offset, frame_data *fd, proto_tree
> *tree)
> {
>   struct amf_hdr *dp;
>   int len;
>   char to_slot[255];
>   char from_slot[255];
>   char to_msa[255];
>   char to_mae[255];
>   char from_msa[255];
>   char from_mae[255];
>   char priority[255];
>   char length[255];
> 
>   dp = (struct amf_hdr *)&pd[offset];

As Gilbert notes, casting "&pd[offset]" to a pointer to a particular
structure, and then dereferencing the pointer, isn't a good idea, as
there is no guarantee that "&pd[offset]" is properly aligned, and if any
of those fields are integral values longer than one byte, and aren't
aligned on the natural boundary, the attempt to refer to them will cause
a crash on a number of processors.

In addition, if any of the fields in the structure are longer than one
byte long, the *byte order* of those fields will matter, and you'll want
to refer to them in the byte order they have in the *protocol*, not in
the byte order on the machine on which you're running Ethereal, as
Ethereal runs on machines of both byte orders, and so there is a
reasonable chance that Ethereal will get incorrect values for those
fields on some machines.

As he also noted, you should be using tvbuffs.  I would suggest that you
do something such as

	/*
	 * Offsets of fields in an AMF header.
	 */
	#define FROM_SLOT	0
	#define	TO_SLOT		2
	#define FROM_MSA	4

and so on, #defining values for the offsets within a structure (the
above code is assuming that those fields are all 2-byte fields; you'd
obviously put the correct offset values in there), and do stuff such as

	to_slot_val = tvb_get_ntohs(tvb, TO_SLOT);

to fetch the value (that code assumes that the "to_slot" field is a
2-byte big-endian quantity; use "tvb_get_guint8()" if it's 1-byte,
"tvb_get_letohs()" if it's 2-byte little-endian, "tvb_get_ntohl()" if
it's 4-byte big-endian, and "tvb_get_letohl()" if it's little-endian.