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 5366] [PATCH] Proper dissection for Tight VNC negotiation

Date: Fri, 19 Nov 2010 10:57:47 -0800 (PST)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5366

Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #12 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2010-11-19 10:57:46 PST ---
(In reply to comment #8)
> (In reply to comment #7)
> > I was pretty sure I checked it before reporting it here, but I now double
> > checked it by reverting packet-vnc.c to SVN and running the same test.  The SVN
> > version does NOT show the problem.  (I didn't investigate further.)
> 
> Indeed - but it's exactly as I suspected - it's not purely my fault, as my
> improved dissection actually exposes this bug. Up until now you couldn't reach
> that path, because it would fail earlier, on mis-dissecting or not dissecting
> the Tight message exchange.
> 
> It all begins on packet 34 being mis-dissected, offset 03e6 - it should
> continue to dissect another rectangle. For it to properly do it, it should
> desegment first, which it didn't do.
> 
> So an ugly hack that fixes some of the paths is:
>     if (num_rects > 10000) {
>         /* this is a bug in the dissector, misdissecting the packet, but lets
> not run into an infinite loop */
>         DISSECTOR_ASSERT_NOT_REACHED();
>         return 0;

Well, frame 34 says there are 2 rectangles and 138(?) sub-rectangles and fully
dissects them all--but there's more data after that.  Not sure what it is.

Anyway, I checked in a variation of the above change and a bunch of other
protections in rev 34977.  Please check it out and let me know if I broke
anything--things still look broken, but it at least *looks* like progress is
being made (anything that gets rid of all of those assertions must be called
progress!).

Thanks for the contribution!

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