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
