morriss@xxxxxxxxxxxxx wrote:
> http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=33464
>
> User: morriss
> Date: 2010/07/07 08:52 AM
>
> Log:
> Fix infinite recursion reported in https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=4984 : In try_get_ber_length() make sure we move forward before recursing.
>
> Directory: /trunk/epan/dissectors/
> Changes Path Action
> +57 -56 packet-ber.c Modified
If I apply the attached debugging code to packet-ber.c I see some high
stack counts in the fuzz capture from bug 4984. It looks like we're
still vulnerable to a stack overflow.
Shouldn't we remove recursion from try_get_ber_length or at least throw
an exception when we run into, say, 5 levels of nesting?
It looks like plugins/asn1/packet-asn1.c has a similar problem.
Index: epan/dissectors/packet-ber.c
===================================================================
--- epan/dissectors/packet-ber.c (revision 33465)
+++ epan/dissectors/packet-ber.c (working copy)
@@ -948,6 +948,7 @@
* an indefinite length and haven't reached EOC.
*/
/* 8.1.3 Length octets */
+static int tgbl_stack_height;
static gboolean
try_get_ber_length(tvbuff_t *tvb, int *bl_offset, gboolean pc, guint32 *length, gboolean *ind) {
int offset = *bl_offset;
@@ -965,6 +966,7 @@
oct = tvb_get_guint8(tvb, offset);
offset += 1;
+tgbl_stack_height++;
if(!(oct&0x80)) {
/* 8.1.3.4 */
tmp_length = oct;
@@ -1029,10 +1031,12 @@
save_pc = last_pc;
save_tag = last_tag;
+tgbl_stack_height = 0;
if(!try_get_ber_length(tvb, &bl_offset, last_pc, &bl_length, ind)) {
/* we couldn't get a length */
bl_offset = offset;
}
+if (tgbl_stack_height > 10) fprintf(stderr, "stack height: %d\n", tgbl_stack_height);
if (length)
*length = bl_length;
