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 3922] TN5250 Dissector

Date: Fri, 4 Dec 2009 18:05:14 -0800 (PST)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3922

--- Comment #8 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2009-12-04 18:05:11 PST ---
Sorry, I was looking at this a while ago but haven't had much time lately. I
hadn't finished reviewing the latest patch, but I did have a couple of comments
(that I had been saving until I finished, but, well, I'll send them now since I
don't know when I can get back to this):

This:

  while (tvb_reported_length_remaining(tvb, offset) > 0 && !done) {

        [...]
    length = tvb_get_guint8(tvb,offset);
        [...]
    offset += (length - 6);
  }

Could loop forever if length turns out to be less than 6.  There are some other
loops like this--they probably need some sanity checks on the length retrieved
out of the TVB so we can be sure that offset is always increasing.

Other loops rely on tn5250_add_hf_items() increasing the offset which in turn
relies on the length in the hf_item being set correctly.  It would probably
be good (since there are so many of those things) to put a DISSECTOR_ASSERT()
or
something in add_hf_items() to make sure the length is positive; we wouldn't
want a transposed 0 and 1 to cause a loop.

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