Ethereal-dev: Re: [Ethereal-dev] ethereal segfault

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

From: Guy Harris <gharris@xxxxxxxxx>
Date: Sun, 16 Jan 2005 19:56:13 -0800
Guy Harris wrote:

If it is, the problem is that the data for the tvbuff for the reassembled chunks starts with the *second* byte of the reassembled data, so that when g_free() is called with the pointer to the data in the tvbuff, the argument doesn't point to the beginning of the mallocated chunk of data, so it frees up a bogus pointer. In OS X, I get a lot of warnings about attempts to free a non-mallocated pointer; in other OSes, it might cause a crash, if free() (which g_free() calls) doesn't check for a non-mallocated pointer.

I've checked in a change that *should* handle it. I don't have the DNP spec (USD 200 to join is a bit much just to check this one thing, and USD 400 for a printed documentation binder is definitely a bit much), but I'm inferring that:

the data, starting at the transport byte, consists of a bunch of chunks of up to 16 bytes, with each chunk followed by a CRC;

the first chunk includes the transport layer byte, so that the CRC for the first chunk covers the transport layer byte;

the application-layer data, constructed by concatenating the data portions of the chunks, should *not* include the transport layer byte.

The change just omits the transport layer byte in the first chunk when filling in the blob of memory (but not when computing the CRC).

I also put in a couple of XXX comments:

1) the dissector should check for dl_len being <= 5 - if it's < 5, the packet is presumably bogus, and if it's 5, that presumably means there's no application layer data;

2) should "data_len" be computed as "dl_len - 6", as the transport layer byte is no longer being included in the tvbuff, with "data_len" having "chk_size - data_offset", rather than "chk_size", subtracted from it? (The variable name "data_offset" comes from the version I checked in; it's the offset from "offset" of the first application layer data byte, and is 1 in the first iteration of the loop, so that the transport layer byte is skipped, and is 0 in all subsequent iterations.)

That should fix the segfault problem, so I didn't remove the DNP3 dissector from the build.