ANNOUNCEMENT: Live Wireshark University & Allegro Packets online APAC Wireshark Training Session
April 17th, 2024 | 14:30-16:00 SGT (UTC+8) | Online

Wireshark-bugs: [Wireshark-bugs] [Bug 5469] fix for malformed VNC handshake

Date: Sat, 31 Dec 2011 09:52:39 -0800 (PST)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5469

Michael Mann <mmann78@xxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mmann78@xxxxxxxxxxxx

--- Comment #8 from Michael Mann <mmann78@xxxxxxxxxxxx> 2011-12-31 09:52:37 PST ---
(In reply to comment #7)
> (From update of attachment 7471 [details])
> Since the gtk-vnc bug has been fixed, do we bother making any changes at this
> point?  Well, either way, this patch is being rejected since the
> expert_add_info_format() call can't be within an if (tree) block.  And when
> will pinfo ever be NULL?  It is dereferenced all over the place, so if it can
> be NULL, then it would need to be checked in many more places than this.  Also,
> I'm not sure returning TRUE if the data violates the protocol is a good idea
> either.

vnc_is_client_or_server_version_message() has two purposes.  One is for
checking the heuristics to determine if the packet is a VNC packet.  During
that check, the add_expert_info_format() shouldn't be added if the packet is
"malformed".  

The second purpose is for actually dissecting a VNC packet which does need the
add_expert_info_format() to address this bug.  

pinfo = NULL (and tree = NULL) during the "heuristic check", but not during
packet dissection (where tree may equal NULL, but pinfo should never be NULL)

> Regarding the other changes for the minor cleanup, perhaps it would be better
> to open a separate bug report and post a patch with only those changes? 
> Especially since I'm really considering closing this bug as wontfix.

I thought it was still possible to have old implementations that may not have
the gtk-vnc bug fixed, so I think this should still be fixed (with the entire
patch accepted)

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