Re: [Wireshark-commits] rev 33464: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-ber.c

[Gerald Combs <gerald () wireshark org>]

morriss () wireshark org 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;
 

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe