i have checked in a fix for bug 3,
just a simple g_assert(length<0xff) which should be enough here.
thankyou for your input and analysis of culnerabilities.
i will, unless someone beats me to it, look into bug 1 and 2 tomorrow.
thanks.
On Mon, 14 Mar 2005 18:54:53 -0800, Bryan Fulton <bryan@xxxxxxxxxxxx> wrote:
> 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
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>
