Wireshark-bugs: [Wireshark-bugs] [Bug 7046] Enhancement to LDP dissector to support changes prop
Date: Mon, 11 Jun 2012 09:48:10 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7046

--- Comment #7 from Jeff Morriss <[email protected]> 2012-06-11 09:48:09 PDT ---
(In reply to comment #6)
> (In reply to comment #5)
> > These changes introduced (or made worse) a static overrun (detected as Coverty
> > ID 703472): there are two places where dissect_subtlv_interface_parameters() is
> > called with 2 different interface_params_header_fields[] parameters but this
> > patch only added the flow label changes to the first.  In the 2nd case if a
> > flow label parameter is detected it could access entries 36, 37, and 38 of the
> > passed in interface_params_header_fields[] (which don't exist):
> > 
> > 2871    case FEC_VC_INTERFACEPARAM_FLOWLABEL:
> > 2872        proto_item_append_text(ti,": Flow Label for Pseudowire");
> > 2873        proto_tree_add_item(vcintparam_tree, *interface_parameters_hf[36],
> > tvb, offset+2, 1, ENC_BIG_ENDIAN);
> > 2874        proto_tree_add_item(vcintparam_tree, *interface_parameters_hf[37],
> > tvb, offset+2, 1, ENC_BIG_ENDIAN);
> > 2875        proto_tree_add_item(vcintparam_tree, *interface_parameters_hf[38],
> > tvb, offset+2, 2, ENC_BIG_ENDIAN);
> > 
> > I could mechanically add a few entries, but do you know this protocol well
> > enough to make a more informed fix?
> 
> Hey Jeff, i can take a look at it. can you give me some details on the issue so
> that i can test the fix?

Well, basically the problem is as described: there are 2
interface_params_header_fields[] but they have different numbers of entries in
each array.  They are both passed into dissect_subtlv_interface_parameters()
which could access past the end of the shorter of the 2 arrays (IIRC the
shorter has ~32 entries but as you can see from the quoted code the function
could access up to the 38th entry; of course this could cause a segmentation
violation).

It may be that the code logically can't happen (e.g., the function is never
called with the shorter array when there's a FEC_FC_INTERFACEPARAM_FLOWLABEL
there) but I couldn't determine that quickly.  Also keep in mind that Wireshark
could be looking at an invalid packet (where the normal rules of the protocol
do not apply).

(I do find it interesting that fuzz testing hasn't found this problem yet.)

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