osmocom
/
wireshark
11
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

T125: avoid returning from TRY/CATCH in dissect_t125_heur 

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>
This commit is contained in:
Peter Wu 2018-10-09 17:23:44 +02:00 committed by Anders Broman
parent 1a9f074c0c
commit d9231144b6
2 changed files with 50 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
epan/dissectors/asn1/t125/packet-t125-template.c
View 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;
}

58
epan/dissectors/packet-t125.c
View 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 */