Wireshark · Ethereal-dev: Re: [Ethereal-dev] [Coverity] Possible Format String Vulnerabilites

[Gerald Combs <gerald@xxxxxxxxxxxx>]

metatech wrote:
>>Let me know if these look valid, and if so, when a possible patch would
>>be made available. We appreciate your feedback as a method to improve
>>and expand our security checkers.
> 
>>Bug 1:
>>/ethereal-0.10.10/epan/dissectors/packet-mq.c:dissect_mq_pdu
>>- sStructId pulled off of tvb via tvb_get_string() and passed to
>>proto_tree_add_text() as format argument.
> 
> Bryan,
> 
> Beware that at line 1595 there is a enumeration of all possible values
> for the StructId,
> so the the StructId is only pulled off the packet and taken into account
> if found in a known list.

I think the problem comes from the use of sStructId (and not StructID)
in lines 1609 and 1610:

	sStructId = tvb_get_string(tvb, offset, 4);
	ti = proto_tree_add_text(mqroot_tree, tvb, offset, -1,
		(const char*)sStructId);

If we read "a string containing %s" into sStructId, proto_tree_add_text
assumes that there is a string argument to follow when there isn't.  A
fix has been checked in.

BTW, is there any way to check for this sort of thing at compile time?
proto_tree_add_text() should arguably have at least 6 arguments no
matter what.