T125: avoid returning from TRY/CATCH in dissect_t125_heur
authorPeter Wu <peter@lekensteyn.nl>
Tue, 9 Oct 2018 15:23:44 +0000 (17:23 +0200)
committerAnders Broman <a.broman58@gmail.com>
Wed, 10 Oct 2018 04:07:05 +0000 (04:07 +0000)
Doing so corrupts the exceptions stack and causes crashes elsewhere.
Move the heuristics check after get_ber_identifier as dissect_t125
calls that check too.

Bug: 15189
Change-Id: I816fcd693141c5e9e2979348f58bf5a8112290da
Fixes: v2.9.0rc0-2122-gf710f21833 ("T125: Add a heuristic test case.")
Reviewed-on: https://code.wireshark.org/review/30096
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Émilio Gonzalez <egg997@gmail.com>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
epan/dissectors/asn1/t125/packet-t125-template.c
epan/dissectors/packet-t125.c

index 7212c5b3b7c7a9215a9cb2fa56c86a4283d5675c..4edde79f481528d953e94bde40d9e0df0dc7d63d 100644 (file)
@@ -88,7 +88,6 @@ dissect_t125_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, vo
   gboolean pc;
   gint32 tag;
   volatile gboolean failed;
-  gboolean is_t125;
 
   /*
    * We must catch all the "ran past the end of the packet" exceptions
@@ -99,45 +98,41 @@ dissect_t125_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, vo
    */
   failed = FALSE;
   TRY {
-    /*
-     * Check that the first byte of the packet is a valid t125/MCS header.
-     * This might not be enough, but since t125 only catch COTP packets,
-     * it should not be a problem.
-     */
-    guint8 first_byte = tvb_get_guint8(tvb, 0) >> 2;
-    switch (first_byte) {
-      case HF_T125_ERECT_DOMAIN_REQUEST:
-      case HF_T125_ATTACH_USER_REQUEST:
-      case HF_T125_ATTACH_USER_CONFIRM:
-      case HF_T125_CHANNEL_JOIN_REQUEST:
-      case HF_T125_CHANNEL_JOIN_CONFIRM:
-      case HF_T125_DISCONNECT_PROVIDER_ULTIMATUM:
-      case HF_T125_SEND_DATA_REQUEST:
-      case HF_T125_SEND_DATA_INDICATION:
-        is_t125 = TRUE;
-        break;
-      default:
-        is_t125 = FALSE;
-        break;
-    }
-    if(is_t125) {
-      dissect_t125(tvb, pinfo, parent_tree, NULL);
-      return TRUE;
-    }
-
     /* could be BER */
     get_ber_identifier(tvb, 0, &ber_class, &pc, &tag);
   } CATCH_BOUNDS_ERRORS {
     failed = TRUE;
   } ENDTRY;
 
-  /* is this strong enough ? */
-  if (!failed && ((ber_class==BER_CLASS_APP) && ((tag>=101) && (tag<=104)))) {
+  if (failed) {
+      return FALSE;
+  }
+
+  if (((ber_class==BER_CLASS_APP) && ((tag>=101) && (tag<=104)))) {
     dissect_t125(tvb, pinfo, parent_tree, NULL);
 
     return TRUE;
   }
 
+  /*
+   * Check that the first byte of the packet is a valid t125/MCS header.
+   * This might not be enough, but since t125 only catch COTP packets,
+   * it should not be a problem.
+   */
+  guint8 first_byte = tvb_get_guint8(tvb, 0) >> 2;
+  switch (first_byte) {
+    case HF_T125_ERECT_DOMAIN_REQUEST:
+    case HF_T125_ATTACH_USER_REQUEST:
+    case HF_T125_ATTACH_USER_CONFIRM:
+    case HF_T125_CHANNEL_JOIN_REQUEST:
+    case HF_T125_CHANNEL_JOIN_CONFIRM:
+    case HF_T125_DISCONNECT_PROVIDER_ULTIMATUM:
+    case HF_T125_SEND_DATA_REQUEST:
+    case HF_T125_SEND_DATA_INDICATION:
+      dissect_t125(tvb, pinfo, parent_tree, NULL);
+      return TRUE;
+  }
+
   return FALSE;
 }
 
index d3fa7b9fc28cdf9fcb2b539480e51330275bebf5..4abe431d127a2812d65c7c4f0217af17447fe3d1 100644 (file)
@@ -24,7 +24,6 @@
 #include <epan/exceptions.h>
 
 #include <epan/asn1.h>
-#include <epan/conversation.h>
 #include "packet-ber.h"
 #include "packet-per.h"
 
@@ -420,7 +419,6 @@ dissect_t125_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, vo
   gboolean pc;
   gint32 tag;
   volatile gboolean failed;
-  gboolean is_t125;
 
   /*
    * We must catch all the "ran past the end of the packet" exceptions
@@ -431,45 +429,41 @@ dissect_t125_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, vo
    */
   failed = FALSE;
   TRY {
-    /*
-     * Check that the first byte of the packet is a valid t125/MCS header.
-     * This might not be enough, but since t125 only catch COTP packets,
-     * it should not be a problem.
-     */
-    guint8 first_byte = tvb_get_guint8(tvb, 0) >> 2;
-    switch (first_byte) {
-      case HF_T125_ERECT_DOMAIN_REQUEST:
-      case HF_T125_ATTACH_USER_REQUEST:
-      case HF_T125_ATTACH_USER_CONFIRM:
-      case HF_T125_CHANNEL_JOIN_REQUEST:
-      case HF_T125_CHANNEL_JOIN_CONFIRM:
-      case HF_T125_DISCONNECT_PROVIDER_ULTIMATUM:
-      case HF_T125_SEND_DATA_REQUEST:
-      case HF_T125_SEND_DATA_INDICATION:
-        is_t125 = TRUE;
-        break;
-      default:
-        is_t125 = FALSE;
-        break;
-    }
-    if(is_t125) {
-      dissect_t125(tvb, pinfo, parent_tree, NULL);
-      return TRUE;
-    }
-
     /* could be BER */
     get_ber_identifier(tvb, 0, &ber_class, &pc, &tag);
   } CATCH_BOUNDS_ERRORS {
     failed = TRUE;
   } ENDTRY;
 
-  /* is this strong enough ? */
-  if (!failed && ((ber_class==BER_CLASS_APP) && ((tag>=101) && (tag<=104)))) {
+  if (failed) {
+      return FALSE;
+  }
+
+  if (((ber_class==BER_CLASS_APP) && ((tag>=101) && (tag<=104)))) {
     dissect_t125(tvb, pinfo, parent_tree, NULL);
 
     return TRUE;
   }
 
+  /*
+   * Check that the first byte of the packet is a valid t125/MCS header.
+   * This might not be enough, but since t125 only catch COTP packets,
+   * it should not be a problem.
+   */
+  guint8 first_byte = tvb_get_guint8(tvb, 0) >> 2;
+  switch (first_byte) {
+    case HF_T125_ERECT_DOMAIN_REQUEST:
+    case HF_T125_ATTACH_USER_REQUEST:
+    case HF_T125_ATTACH_USER_CONFIRM:
+    case HF_T125_CHANNEL_JOIN_REQUEST:
+    case HF_T125_CHANNEL_JOIN_CONFIRM:
+    case HF_T125_DISCONNECT_PROVIDER_ULTIMATUM:
+    case HF_T125_SEND_DATA_REQUEST:
+    case HF_T125_SEND_DATA_INDICATION:
+      dissect_t125(tvb, pinfo, parent_tree, NULL);
+      return TRUE;
+  }
+
   return FALSE;
 }
 
@@ -584,7 +578,7 @@ void proto_register_t125(void) {
         NULL, HFILL }},
 
 /*--- End of included file: packet-t125-hfarr.c ---*/
-#line 151 "./asn1/t125/packet-t125-template.c"
+#line 146 "./asn1/t125/packet-t125-template.c"
   };
 
   /* List of subtrees */
@@ -601,7 +595,7 @@ void proto_register_t125(void) {
     &ett_t125_ConnectMCSPDU,
 
 /*--- End of included file: packet-t125-ettarr.c ---*/
-#line 157 "./asn1/t125/packet-t125-template.c"
+#line 152 "./asn1/t125/packet-t125-template.c"
   };
 
   /* Register protocol */