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

[Bryan Fulton <bryan@xxxxxxxxxxxx>]

I didn't mean to tease; here's an example of one of the misused scalar
bugs. Again take a peek and let me know if this seems valid, and we can
send over some more for your inspection.

Thanks again,
.:bryan


Bug 3:
/ethereal-0.10.10/epan/dissectors/packet-manolito.c:dissect_manolito
- length pulled off a tvb via tvb_get_guint8(), and length+1 is
passed to malloc without proper bounds checking. Possible integer
overflow on the allocation, and buffer overflow on the tvb_memcpy(). 


Function "tvb_get_guint8" returns TAINTED data
Variable "length" TAINTED from assignment to tainted return value of
"tvb_get_guint8"

184    length = tvb_get_guint8(tvb, ++offset)

TAINTED variable "(length + 1)" was passed to a tainted sink.

187 	data = malloc(length + 1);

TAINTED variable "length" was passed to a tainted sink.

188 	tvb_memcpy(tvb, (guint8*)data, ++offset, length);

...

196 
197	if (dtype == MANOLITO_STRING) 
198     {

TAINTED variable "length" used as index to pointer "data"

199              data[length] = 0;








On Mon, 2005-03-14 at 18:38, Bryan Fulton wrote:
> Hi,
> 
> While testing our security checkers here at Coverity, we discovered a
> handful of possible flaws in Ethereal-10.10. Most are improper usage of
> scalars pulled off a packet (similar to the recent advisories) and we'll
> send you more detailed information on those soon. Right now, I thought
> you would be interested in a couple of possible format string
> vulnerabilities.  
> 
> 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.
> 
> Thanks,
> .:Bryan Fulton of Coverity, Inc.
> 
> 
> 
> 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.
> 
> 1608 	guint8* sStructId;
> 
> Function "tvb_get_string" returns TAINTED string content
> Variable "sStructId" TAINTED from assignment to tainted return value of 
> "tvb_get_string"
> 
> 1609    sStructId = tvb_get_string(tvb, offset, 4);
> 
> TAINTED string "sStructId" was passed to a format string sink.
> 
> 1610    ti = proto_tree_add_text(mqroot_tree, tvb, offset, -1, 
>                                  (const char*)sStructId);
> 
> 
> 
> Bug 2:
> /ethereal-0.10.10/epan/dissectors/packet-ansi_a.c:elem_cld_party_ascii_num
> - poctets pulled of of tcv via tvb_get_string() and passed to 
> proto_tree_add_string_function() as format argument.
> - This one is fairly subtle to notice as first glance. It looks like the
> value argument is missing, as the string "Digits: %s" is used as value
> in the call to proto_tree_add_string_format(). The user-controlled
> poctets is then unsafely passed as the FS argument.
> 
> Function "tvb_get_string" returns TAINTED string content
> Variable "poctets" TAINTED from assignment to tainted return value of 
> "tvb_get_string"
> 
> 5307 	    poctets = tvb_get_string(tvb, curr_offset, len - (curr_offset
> - offset));
> 
> TAINTED string "poctets" was passed to a format string sink. 
> 5309 	 proto_tree_add_string_format(tree,                             
> hf_ansi_a_cld_party_ascii_num,
> 5310 		tvb, curr_offset, len - (curr_offset - offset),
> 5311 		"Digits: %s",
> 5312 		poctets);
-- 
Bryan J Fulton
Coverity, Inc.
185 Berry St, Suite 3600
San Francisco, CA 94107
www.coverity.com

Phone: 415-321-5206
Fax:   415-541-9521
Email: bryan@xxxxxxxxxxxx