From f434820705b7875e5eaab1bba77c0264b1eb1bd6 Mon Sep 17 00:00:00 2001 From: Pascal Quantin Date: Mon, 29 May 2017 21:49:26 +0200 Subject: TCAP: fix SRT analysis When reviewing the code, the following issues were identified: - otid/dtid on 3 bytes were not stored - when receiving the first continue from dest, the TC_END hash entry was created with the source tid / address instead of destination ones - when receiving the first continue from src, the logic could prevent the creation of the hash entry Bug: 13739 Change-Id: If4ee70f0fa69f5ff74fdf75f3a741102baa0121a Reviewed-on: https://code.wireshark.org/review/21780 Petri-Dish: Pascal Quantin Tested-by: Petri Dish Buildbot Reviewed-by: Pascal Quantin Reviewed-by: Michael Mann --- epan/dissectors/asn1/tcap/packet-tcap-template.c | 17 ++++++++--- epan/dissectors/asn1/tcap/tcap.cnf | 6 ++++ epan/dissectors/packet-tcap.c | 39 ++++++++++++++++-------- 3 files changed, 44 insertions(+), 18 deletions(-) diff --git a/epan/dissectors/asn1/tcap/packet-tcap-template.c b/epan/dissectors/asn1/tcap/packet-tcap-template.c index 24e3f58b11..576f78da9d 100644 --- a/epan/dissectors/asn1/tcap/packet-tcap-template.c +++ b/epan/dissectors/asn1/tcap/packet-tcap-template.c @@ -1036,6 +1036,7 @@ tcaphash_cont_matching(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, proto_item *pi; proto_item *stat_item=NULL; proto_tree *stat_tree=NULL; + gboolean use_dst = TRUE; #ifdef DEBUG_TCAPSRT dbg(51,"src %s srcTid %lx dst %s dstTid %lx ", address_to_str(wmem_packet_scope(), &pinfo->src), p_tcapsrt_info->src_tid, address_to_str(wmem_packet_scope(), &pinfo->dst), p_tcapsrt_info->dst_tid); @@ -1091,11 +1092,13 @@ tcaphash_cont_matching(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, #endif p_tcaphash_begincall = find_tcaphash_begin(&tcaphash_begin_key, pinfo, FALSE); if(!p_tcaphash_begincall){ + try_src: /* can this actually happen? */ #ifdef DEBUG_TCAPSRT dbg(12,"BNotFound trying stid,src"); #endif /* Do we have a continue from the same source? (stid,src) */ + use_dst = FALSE; tcaphash_begin_key.tid = p_tcapsrt_info->src_tid; if (pinfo->src.type == ss7pc_address_type && pinfo->dst.type == ss7pc_address_type) { @@ -1128,21 +1131,21 @@ tcaphash_cont_matching(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, create_tcaphash_cont(&tcaphash_cont_key, p_tcaphash_begincall->context); - /* Create END for (stid,src) */ - tcaphash_end_key.tid = p_tcapsrt_info->src_tid; + /* Create END for (dtid,dst) or (stid,src) */ + tcaphash_end_key.tid = use_dst ? p_tcapsrt_info->dst_tid : p_tcapsrt_info->src_tid; if (pinfo->src.type == ss7pc_address_type && pinfo->dst.type == ss7pc_address_type) { /* We have MTP3 PCs (so we can safely do this cast) */ - tcaphash_end_key.pc_hash = mtp3_pc_hash((const mtp3_addr_pc_t *)pinfo->src.data); + tcaphash_end_key.pc_hash = mtp3_pc_hash((const mtp3_addr_pc_t *)(use_dst ? pinfo->dst.data : pinfo->src.data)); } else { /* Don't have MTP3 PCs (have SCCP GT ?) */ - tcaphash_end_key.pc_hash = g_str_hash(address_to_str(wmem_packet_scope(), &pinfo->src)); + tcaphash_end_key.pc_hash = g_str_hash(address_to_str(wmem_packet_scope(), use_dst ? &pinfo->dst : &pinfo->src)); } tcaphash_end_key.hashKey=tcaphash_end_calchash(&tcaphash_end_key); #ifdef DEBUG_TCAPSRT dbg(10,"New Ekey %lx ",tcaphash_end_key.hashKey); - dbg(51,"addr %s ", address_to_str(wmem_packet_scope(), &pinfo->src)); + dbg(51,"addr %s ", address_to_str(wmem_packet_scope(), use_dst ? &pinfo->dst : &pinfo->src)); dbg(51,"Tid %lx ",tcaphash_end_key.tid); dbg(11,"Frame reqlink #%u ", pinfo->num); #endif @@ -1153,6 +1156,10 @@ tcaphash_cont_matching(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, #ifdef DEBUG_TCAPSRT dbg(12,"BnotFound "); #endif + if (use_dst) { + /* make another try with src tid / address */ + goto try_src; + } } /* begin found */ } /* cont found */ /* display tcap session, if available */ diff --git a/epan/dissectors/asn1/tcap/tcap.cnf b/epan/dissectors/asn1/tcap/tcap.cnf index 217048ab82..8898561d4a 100644 --- a/epan/dissectors/asn1/tcap/tcap.cnf +++ b/epan/dissectors/asn1/tcap/tcap.cnf @@ -156,6 +156,9 @@ ABRT-apdu/_untag/user-information abrt_user_information case 2: gp_tcapsrt_info->src_tid=tvb_get_ntohs(parameter_tvb, 0); break; + case 3: + gp_tcapsrt_info->src_tid=tvb_get_ntoh24(parameter_tvb, 0); + break; case 4: gp_tcapsrt_info->src_tid=tvb_get_ntohl(parameter_tvb, 0); break; @@ -201,6 +204,9 @@ ABRT-apdu/_untag/user-information abrt_user_information case 2: gp_tcapsrt_info->dst_tid=tvb_get_ntohs(parameter_tvb, 0); break; + case 3: + gp_tcapsrt_info->dst_tid=tvb_get_ntoh24(parameter_tvb, 0); + break; case 4: gp_tcapsrt_info->dst_tid=tvb_get_ntohl(parameter_tvb, 0); break; diff --git a/epan/dissectors/packet-tcap.c b/epan/dissectors/packet-tcap.c index 2c68368c00..da884c0328 100644 --- a/epan/dissectors/packet-tcap.c +++ b/epan/dissectors/packet-tcap.c @@ -783,6 +783,9 @@ dissect_tcap_OrigTransactionID(gboolean implicit_tag _U_, tvbuff_t *tvb _U_, int case 2: gp_tcapsrt_info->src_tid=tvb_get_ntohs(parameter_tvb, 0); break; + case 3: + gp_tcapsrt_info->src_tid=tvb_get_ntoh24(parameter_tvb, 0); + break; case 4: gp_tcapsrt_info->src_tid=tvb_get_ntohl(parameter_tvb, 0); break; @@ -816,7 +819,7 @@ static const ber_sequence_t Begin_sequence[] = { static int dissect_tcap_Begin(gboolean implicit_tag _U_, tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_, proto_tree *tree _U_, int hf_index _U_) { -#line 222 "./asn1/tcap/tcap.cnf" +#line 228 "./asn1/tcap/tcap.cnf" gp_tcapsrt_info->ope=TC_BEGIN; /* Do not change col_add_str() to col_append_str() here: we _want_ this call @@ -838,7 +841,7 @@ gp_tcapsrt_info->ope=TC_BEGIN; static int dissect_tcap_DestTransactionID(gboolean implicit_tag _U_, tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_, proto_tree *tree _U_, int hf_index _U_) { -#line 179 "./asn1/tcap/tcap.cnf" +#line 182 "./asn1/tcap/tcap.cnf" tvbuff_t *parameter_tvb; guint8 len , i; proto_tree *subtree; @@ -866,6 +869,9 @@ dissect_tcap_DestTransactionID(gboolean implicit_tag _U_, tvbuff_t *tvb _U_, int case 2: gp_tcapsrt_info->dst_tid=tvb_get_ntohs(parameter_tvb, 0); break; + case 3: + gp_tcapsrt_info->dst_tid=tvb_get_ntoh24(parameter_tvb, 0); + break; case 4: gp_tcapsrt_info->dst_tid=tvb_get_ntohl(parameter_tvb, 0); break; @@ -898,7 +904,7 @@ static const ber_sequence_t End_sequence[] = { static int dissect_tcap_End(gboolean implicit_tag _U_, tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_, proto_tree *tree _U_, int hf_index _U_) { -#line 236 "./asn1/tcap/tcap.cnf" +#line 242 "./asn1/tcap/tcap.cnf" gp_tcapsrt_info->ope=TC_END; col_set_str(actx->pinfo->cinfo, COL_INFO, "End "); @@ -920,7 +926,7 @@ static const ber_sequence_t Continue_sequence[] = { static int dissect_tcap_Continue(gboolean implicit_tag _U_, tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_, proto_tree *tree _U_, int hf_index _U_) { -#line 243 "./asn1/tcap/tcap.cnf" +#line 249 "./asn1/tcap/tcap.cnf" gp_tcapsrt_info->ope=TC_CONT; col_set_str(actx->pinfo->cinfo, COL_INFO, "Continue "); @@ -991,7 +997,7 @@ static const ber_sequence_t Abort_sequence[] = { static int dissect_tcap_Abort(gboolean implicit_tag _U_, tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_, proto_tree *tree _U_, int hf_index _U_) { -#line 250 "./asn1/tcap/tcap.cnf" +#line 256 "./asn1/tcap/tcap.cnf" gp_tcapsrt_info->ope=TC_ABORT; col_set_str(actx->pinfo->cinfo, COL_INFO, "Abort "); @@ -2313,6 +2319,7 @@ tcaphash_cont_matching(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, proto_item *pi; proto_item *stat_item=NULL; proto_tree *stat_tree=NULL; + gboolean use_dst = TRUE; #ifdef DEBUG_TCAPSRT dbg(51,"src %s srcTid %lx dst %s dstTid %lx ", address_to_str(wmem_packet_scope(), &pinfo->src), p_tcapsrt_info->src_tid, address_to_str(wmem_packet_scope(), &pinfo->dst), p_tcapsrt_info->dst_tid); @@ -2368,11 +2375,13 @@ tcaphash_cont_matching(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, #endif p_tcaphash_begincall = find_tcaphash_begin(&tcaphash_begin_key, pinfo, FALSE); if(!p_tcaphash_begincall){ + try_src: /* can this actually happen? */ #ifdef DEBUG_TCAPSRT dbg(12,"BNotFound trying stid,src"); #endif /* Do we have a continue from the same source? (stid,src) */ + use_dst = FALSE; tcaphash_begin_key.tid = p_tcapsrt_info->src_tid; if (pinfo->src.type == ss7pc_address_type && pinfo->dst.type == ss7pc_address_type) { @@ -2405,21 +2414,21 @@ tcaphash_cont_matching(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, create_tcaphash_cont(&tcaphash_cont_key, p_tcaphash_begincall->context); - /* Create END for (stid,src) */ - tcaphash_end_key.tid = p_tcapsrt_info->src_tid; + /* Create END for (dtid,dst) or (stid,src) */ + tcaphash_end_key.tid = use_dst ? p_tcapsrt_info->dst_tid : p_tcapsrt_info->src_tid; if (pinfo->src.type == ss7pc_address_type && pinfo->dst.type == ss7pc_address_type) { /* We have MTP3 PCs (so we can safely do this cast) */ - tcaphash_end_key.pc_hash = mtp3_pc_hash((const mtp3_addr_pc_t *)pinfo->src.data); + tcaphash_end_key.pc_hash = mtp3_pc_hash((const mtp3_addr_pc_t *)(use_dst ? pinfo->dst.data : pinfo->src.data)); } else { /* Don't have MTP3 PCs (have SCCP GT ?) */ - tcaphash_end_key.pc_hash = g_str_hash(address_to_str(wmem_packet_scope(), &pinfo->src)); + tcaphash_end_key.pc_hash = g_str_hash(address_to_str(wmem_packet_scope(), use_dst ? &pinfo->dst : &pinfo->src)); } tcaphash_end_key.hashKey=tcaphash_end_calchash(&tcaphash_end_key); #ifdef DEBUG_TCAPSRT dbg(10,"New Ekey %lx ",tcaphash_end_key.hashKey); - dbg(51,"addr %s ", address_to_str(wmem_packet_scope(), &pinfo->src)); + dbg(51,"addr %s ", address_to_str(wmem_packet_scope(), use_dst ? &pinfo->dst : &pinfo->src)); dbg(51,"Tid %lx ",tcaphash_end_key.tid); dbg(11,"Frame reqlink #%u ", pinfo->num); #endif @@ -2430,6 +2439,10 @@ tcaphash_cont_matching(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, #ifdef DEBUG_TCAPSRT dbg(12,"BnotFound "); #endif + if (use_dst) { + /* make another try with src tid / address */ + goto try_src; + } } /* begin found */ } /* cont found */ /* display tcap session, if available */ @@ -3200,7 +3213,7 @@ proto_reg_handoff_tcap(void) /*--- End of included file: packet-tcap-dis-tab.c ---*/ -#line 1919 "./asn1/tcap/packet-tcap-template.c" +#line 1926 "./asn1/tcap/packet-tcap-template.c" } static void init_tcap(void); @@ -3542,7 +3555,7 @@ proto_register_tcap(void) NULL, HFILL }}, /*--- End of included file: packet-tcap-hfarr.c ---*/ -#line 1992 "./asn1/tcap/packet-tcap-template.c" +#line 1999 "./asn1/tcap/packet-tcap-template.c" }; /* Setup protocol subtree array */ @@ -3590,7 +3603,7 @@ proto_register_tcap(void) &ett_tcap_Associate_source_diagnostic, /*--- End of included file: packet-tcap-ettarr.c ---*/ -#line 2002 "./asn1/tcap/packet-tcap-template.c" +#line 2009 "./asn1/tcap/packet-tcap-template.c" }; /*static enum_val_t tcap_options[] = { -- cgit v1.2.1