Huge thanks to our Platinum Members Endace and LiveAction,
and our Silver Member Veeam, for supporting the Wireshark Foundation and project.

Wireshark-bugs: [Wireshark-bugs] [Bug 6416] Improved CIP and ENIP dissectors

Date: Thu, 27 Oct 2011 18:16:11 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6416

--- Comment #23 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2011-10-27 18:16:08 PDT ---
(In reply to comment #20)
> (In reply to comment #15)
> > A couple other comments:
> > 1) Shouldn't the tvb_length_remaining() calls be
> > tvb_reported_length_remaining()?  At least the ones checking for how much data
> > is left before raising an expert info--since having a capture cut short by a
> > snaplen is does not a malformed packet make (it makes a truncated packet). 
> > Another related question is whether those checks are really necessary: if they
> > weren't there the dissector would walk off the end of the TVB and throw the
> > appropriate exception.
> 
> I can make the changes for tvb_reported_length_remaining(), but the issue I've
> run into is that when the exception is thrown some already dissected fields
> were "discarded", I think to the top of the subtree where the item that walked
> off the end was being added to.  Since this dissector used to have a larger
> amount of proto_tree_add_texts instead of proto_tree_add_items perhaps that was
> the real reason fields were being discarded, but I was trying to prevent those
> cases again.

That's...  Really weird.  I can't imagine how anything added to the tree would
be "unrolled" due to an exception.

> > 2) Double check the encoding values.  For example hf_cip_link_address_string is
> > used with ENC_LITTLE_ENDIAN; that should probably be ENC_ASCII|ENC_NA.
> 
> I'm going to chalk this one up to the patch not applying cleanly because my
> code (and the patch from what I can see) has hf_cip_link_address_string with a 
> ENC_ASCII|ENC_NA encoding.

Oh boy..  Looks like I was reviewing attachment 7223 which was the one prior to
the fix-encoding-args one.  <sigh> I don't know how that happened (especially
since bugzilla normally hides obsolete attachments). Sorry about that...

BTW, we've been having a rash of fuzz failures from the CIP dissector recently.

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.