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

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

Date Index Thread Index Other Months All Mailing Lists
Date Prev Date Next Thread Prev Thread Next
From: Gerald Combs <gerald@xxxxxxxxxxxx>
Date: Wed, 16 Mar 2005 11:24:57 -0600
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.