diff options
author | Pascal Quantin <pascal.quantin@gmail.com> | 2017-01-27 18:20:40 +0100 |
---|---|---|
committer | Alexis La Goutte <alexis.lagoutte@gmail.com> | 2017-02-01 08:58:14 +0000 |
commit | 5dde07c8fdb2e17c4d4142427fe29f7db66c1965 (patch) | |
tree | 21908b58c79d54f95ae3cfe0c76bd49638ec5e17 /epan/dissectors | |
parent | 275594231dc1f4a08f24123e4413acfd11b59d4e (diff) |
GSM A / NAS-EPS: detect missing mandatory information elements
Current code is not able to detect missing mandatory information elements
because the macro will return once the end of the payload is reached.
Remove this check from all mandatory IE macros, and put it at the beginning
of optional IE ones. It should allow to detect any missing mandatory IE
while still stopping message dissection in case optional IEs are not
present.
Change-Id: Ie820740e25c1d03ee3462fa4a913c3a7870fcc2d
Reviewed-on: https://code.wireshark.org/review/19816
Reviewed-by: Pascal Quantin <pascal.quantin@gmail.com>
Petri-Dish: Pascal Quantin <pascal.quantin@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Diffstat (limited to 'epan/dissectors')
-rw-r--r-- | epan/dissectors/packet-gsm_a_common.h | 20 | ||||
-rw-r--r-- | epan/dissectors/packet-gsm_a_dtap.c | 10 | ||||
-rw-r--r-- | epan/dissectors/packet-gsm_a_rr.c | 2 | ||||
-rw-r--r-- | epan/dissectors/packet-nas_eps.c | 42 |
4 files changed, 8 insertions, 66 deletions
diff --git a/epan/dissectors/packet-gsm_a_common.h b/epan/dissectors/packet-gsm_a_common.h index 91bc4ea0dd..6a259b1307 100644 --- a/epan/dissectors/packet-gsm_a_common.h +++ b/epan/dissectors/packet-gsm_a_common.h @@ -411,7 +411,6 @@ WS_DLL_PUBLIC guint16 elem_v_short(tvbuff_t *tvb, proto_tree *tree, packet_info (EMT_elem_name_addition == NULL) ? "" : EMT_elem_name_addition \ ); \ } \ - if ((signed)curr_len <= 0) return; \ } /* This is a version where the length field can be one or two octets depending * if the extension bit is set or not (TS 48.016 p 10.1.2). @@ -437,7 +436,6 @@ WS_DLL_PUBLIC guint16 elem_v_short(tvbuff_t *tvb, proto_tree *tree, packet_info (EMT_elem_name_addition == NULL) ? "" : EMT_elem_name_addition \ ); \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_MAND_TLV_E(EMT_iei, EMT_pdu_type, EMT_elem_idx, EMT_elem_name_addition, ei_mandatory) \ @@ -458,36 +456,35 @@ WS_DLL_PUBLIC guint16 elem_v_short(tvbuff_t *tvb, proto_tree *tree, packet_info (EMT_elem_name_addition == NULL) ? "" : EMT_elem_name_addition \ ); \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_OPT_TLV(EOT_iei, EOT_pdu_type, EOT_elem_idx, EOT_elem_name_addition) \ {\ + if ((signed)curr_len <= 0) return; \ if ((consumed = elem_tlv(tvb, tree, pinfo, (guint8) EOT_iei, EOT_pdu_type, EOT_elem_idx, curr_offset, curr_len, EOT_elem_name_addition)) > 0) \ { \ curr_offset += consumed; \ curr_len -= consumed; \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_OPT_TELV(EOT_iei, EOT_pdu_type, EOT_elem_idx, EOT_elem_name_addition) \ {\ + if ((signed)curr_len <= 0) return; \ if ((consumed = elem_telv(tvb, tree, pinfo, (guint8) EOT_iei, EOT_pdu_type, EOT_elem_idx, curr_offset, curr_len, EOT_elem_name_addition)) > 0) \ { \ curr_offset += consumed; \ curr_len -= consumed; \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_OPT_TLV_E(EOT_iei, EOT_pdu_type, EOT_elem_idx, EOT_elem_name_addition) \ {\ + if ((signed)curr_len <= 0) return; \ if ((consumed = elem_tlv_e(tvb, tree, pinfo, (guint8) EOT_iei, EOT_pdu_type, EOT_elem_idx, curr_offset, curr_len, EOT_elem_name_addition)) > 0) \ { \ curr_offset += consumed; \ curr_len -= consumed; \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_MAND_TV(EMT_iei, EMT_pdu_type, EMT_elem_idx, EMT_elem_name_addition, ei_mandatory) \ @@ -508,37 +505,36 @@ WS_DLL_PUBLIC guint16 elem_v_short(tvbuff_t *tvb, proto_tree *tree, packet_info (EMT_elem_name_addition == NULL) ? "" : EMT_elem_name_addition \ ); \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_OPT_TV(EOT_iei, EOT_pdu_type, EOT_elem_idx, EOT_elem_name_addition) \ {\ + if ((signed)curr_len <= 0) return; \ if ((consumed = elem_tv(tvb, tree, pinfo, (guint8) EOT_iei, EOT_pdu_type, EOT_elem_idx, curr_offset, EOT_elem_name_addition)) > 0) \ { \ curr_offset += consumed; \ curr_len -= consumed; \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_OPT_TV_SHORT(EOT_iei, EOT_pdu_type, EOT_elem_idx, EOT_elem_name_addition) \ {\ + if ((signed)curr_len <= 0) return; \ if ((consumed = elem_tv_short(tvb, tree, pinfo, EOT_iei, EOT_pdu_type, EOT_elem_idx, curr_offset, EOT_elem_name_addition)) > 0) \ { \ curr_offset += consumed; \ curr_len -= consumed; \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_OPT_T(EOT_iei, EOT_pdu_type, EOT_elem_idx, EOT_elem_name_addition) \ {\ + if ((signed)curr_len <= 0) return; \ if ((consumed = elem_t(tvb, tree, pinfo, (guint8) EOT_iei, EOT_pdu_type, EOT_elem_idx, curr_offset, EOT_elem_name_addition)) > 0) \ { \ curr_offset += consumed; \ curr_len -= consumed; \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_MAND_LV(EML_pdu_type, EML_elem_idx, EML_elem_name_addition) \ @@ -552,7 +548,6 @@ WS_DLL_PUBLIC guint16 elem_v_short(tvbuff_t *tvb, proto_tree *tree, packet_info { \ /* Mandatory, but nothing we can do */ \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_MAND_LV_E(EML_pdu_type, EML_elem_idx, EML_elem_name_addition) \ @@ -566,7 +561,6 @@ WS_DLL_PUBLIC guint16 elem_v_short(tvbuff_t *tvb, proto_tree *tree, packet_info { \ /* Mandatory, but nothing we can do */ \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_MAND_V(EMV_pdu_type, EMV_elem_idx, EMV_elem_name_addition) \ @@ -580,7 +574,6 @@ WS_DLL_PUBLIC guint16 elem_v_short(tvbuff_t *tvb, proto_tree *tree, packet_info { \ /* Mandatory, but nothing we can do */ \ } \ - if ((signed)curr_len <= 0) return; \ } #define ELEM_MAND_VV_SHORT(EMV_pdu_type1, EMV_elem_idx1, EMV_pdu_type2, EMV_elem_idx2) \ @@ -589,7 +582,6 @@ WS_DLL_PUBLIC guint16 elem_v_short(tvbuff_t *tvb, proto_tree *tree, packet_info elem_v_short(tvb, tree, pinfo, EMV_pdu_type2, EMV_elem_idx2, curr_offset, LEFT_NIBBLE); \ curr_offset ++ ; /* consumed length is 1, regardless of contents */ \ curr_len -- ; \ - if ((signed)curr_len <= 0) return; \ } /* diff --git a/epan/dissectors/packet-gsm_a_dtap.c b/epan/dissectors/packet-gsm_a_dtap.c index e8a72566be..4006bbe915 100644 --- a/epan/dissectors/packet-gsm_a_dtap.c +++ b/epan/dissectors/packet-gsm_a_dtap.c @@ -4823,8 +4823,6 @@ dtap_mm_auth_req(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo _U_, guint3 curr_offset++; curr_len--; - if ((signed)curr_len <= 0) return; - ELEM_MAND_V(GSM_A_PDU_TYPE_DTAP, DE_AUTH_PARAM_RAND, " - UMTS challenge or GSM challenge"); ELEM_OPT_TLV(0x20, GSM_A_PDU_TYPE_DTAP, DE_AUTH_PARAM_AUTN, NULL); @@ -4922,8 +4920,6 @@ dtap_mm_cm_reestab_req(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo _U_, curr_offset++; curr_len--; - if ((signed)curr_len <= 0) return; - ELEM_MAND_LV(GSM_A_PDU_TYPE_COMMON, DE_MS_CM_2, NULL); ELEM_MAND_LV(GSM_A_PDU_TYPE_COMMON, DE_MID, NULL); @@ -5069,8 +5065,6 @@ dtap_mm_cm_srvc_req(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo _U_, gui curr_offset++; curr_len--; - if ((signed)curr_len <= 0) return; - ELEM_MAND_LV(GSM_A_PDU_TYPE_COMMON, DE_MS_CM_2, NULL); ELEM_MAND_LV(GSM_A_PDU_TYPE_COMMON, DE_MID, NULL); @@ -5306,8 +5300,6 @@ dtap_mm_loc_upd_req(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo _U_, gui curr_offset++; curr_len--; - if ((signed)curr_len <= 0) return; - ELEM_MAND_V(GSM_A_PDU_TYPE_COMMON, DE_LAI, NULL); ELEM_MAND_V(GSM_A_PDU_TYPE_COMMON, DE_MS_CM_1, NULL); @@ -5540,8 +5532,6 @@ dtap_cc_congestion_control(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo _ curr_offset++; curr_len--; - if ((signed)curr_len <= 0) return; - ELEM_OPT_TLV(0x08, GSM_A_PDU_TYPE_DTAP, DE_CAUSE, NULL); EXTRANEOUS_DATA_CHECK(curr_len, 0, pinfo, &ei_gsm_a_dtap_extraneous_data); diff --git a/epan/dissectors/packet-gsm_a_rr.c b/epan/dissectors/packet-gsm_a_rr.c index d2d548b011..d8840d537b 100644 --- a/epan/dissectors/packet-gsm_a_rr.c +++ b/epan/dissectors/packet-gsm_a_rr.c @@ -10378,8 +10378,6 @@ dtap_rr_paging_resp(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo _U_, gui curr_offset++; curr_len--; - if ((signed)curr_len <= 0) return; - ELEM_MAND_LV(GSM_A_PDU_TYPE_COMMON, DE_MS_CM_2, NULL); ELEM_MAND_LV(GSM_A_PDU_TYPE_COMMON, DE_MID, NULL); diff --git a/epan/dissectors/packet-nas_eps.c b/epan/dissectors/packet-nas_eps.c index 09ded5c0d0..25f5db5abc 100644 --- a/epan/dissectors/packet-nas_eps.c +++ b/epan/dissectors/packet-nas_eps.c @@ -3844,10 +3844,6 @@ nas_emm_detach_req_DL(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo, guint curr_len--; curr_offset++; - /* No more mandatory elements */ - if (curr_len == 0) - return; - /* EMM cause EMM cause 9.9.3.9 O TV 2 */ ELEM_OPT_TV(0x53, NAS_PDU_TYPE_EMM, DE_EMM_CAUSE, NULL); @@ -4119,9 +4115,6 @@ nas_emm_sec_mode_comp(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo, guint pinfo->link_dir = P2P_DIR_UL; - if (curr_len == 0) - return; - /* 23 IMEISV Mobile identity 9.9.2.3 O TLV 11 */ ELEM_OPT_TLV(0x23, NAS_PDU_TYPE_COMMON, DE_EPS_CMN_MOB_ID, " - IMEISV"); @@ -4231,9 +4224,7 @@ nas_emm_trac_area_upd_acc(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo, g /* Fix up the lengths */ curr_len--; curr_offset++; - /* No more mandatory elements */ - if (curr_len == 0) - return; + /* 5A T3412 value GPRS timer 9.9.3.16 O TV 2 */ ELEM_OPT_TV(0x5a, GSM_A_PDU_TYPE_GM, DE_GPRS_TIMER, " - T3412 value"); /* 50 GUTI EPS mobile identity 9.9.3.12 O TLV 13 */ @@ -4496,8 +4487,6 @@ nas_emm_ctrl_plane_serv_req(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo, /* Fix the lengths */ curr_len--; curr_offset++; - if (curr_len == 0) - return; /* 78 ESM message container ESM message container 9.9.3.15 O TLV-E 3-n */ ELEM_OPT_TLV_E(0x78, NAS_PDU_TYPE_EMM, DE_EMM_ESM_MSG_CONT, NULL); @@ -4526,9 +4515,6 @@ nas_emm_serv_accept(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo, guint32 pinfo->link_dir = P2P_DIR_DL; - if (curr_len == 0) - return; - /* 57 EPS bearer context status EPS bearer context status 9.9.2.1 O TLV 4 */ ELEM_OPT_TLV(0x57, NAS_PDU_TYPE_COMMON, DE_EPS_CMN_EPS_BE_CTX_STATUS, NULL); @@ -4549,9 +4535,6 @@ nas_esm_act_ded_eps_bearer_ctx_acc(tvbuff_t *tvb, proto_tree *tree, packet_info guint32 consumed; guint curr_len; - if (len == 0) - return; - curr_offset = offset; curr_len = len; @@ -4662,9 +4645,6 @@ nas_esm_act_def_eps_bearer_ctx_acc(tvbuff_t *tvb, proto_tree *tree, packet_info curr_offset = offset; curr_len = len; - if (len == 0) - return; - /* This message is sent by the UE to the network to acknowledge activation of a default EPS bearer context */ pinfo->link_dir = P2P_DIR_UL; @@ -4921,9 +4901,6 @@ nas_esm_deact_eps_bearer_ctx_acc(tvbuff_t *tvb, proto_tree *tree, packet_info *p curr_offset = offset; curr_len = len; - if (len == 0) - return; - /* This message is sent by the UE to acknowledge deactivation of the EPS bearer context... */ pinfo->link_dir = P2P_DIR_UL; @@ -5010,9 +4987,6 @@ nas_esm_inf_resp(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo, guint32 of curr_offset = offset; curr_len = len; - if (len == 0) - return; - /* This message is sent by the UE to the network in response to an ESM INFORMATION REQUEST... */ pinfo->link_dir = P2P_DIR_UL; @@ -5056,9 +5030,6 @@ nas_esm_mod_eps_bearer_ctx_acc(tvbuff_t *tvb, proto_tree *tree, packet_info *pin curr_offset = offset; curr_len = len; - if (len == 0) - return; - /* This message is sent by the UE to the network to acknowledge the modification of an active EPS bearer context. */ pinfo->link_dir = P2P_DIR_UL; @@ -5111,9 +5082,6 @@ nas_esm_mod_eps_bearer_ctx_req(tvbuff_t *tvb, proto_tree *tree, packet_info *pin curr_offset = offset; curr_len = len; - if (len == 0) - return; - /*This message is sent by the network to inform the UE about events which are relevant for the upper layer... */ pinfo->link_dir = P2P_DIR_DL; @@ -5225,8 +5193,6 @@ nas_esm_pdn_con_req(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo, guint32 /* Fix the lengths */ curr_len--; curr_offset++; - if (curr_len == 0) - return; /* D- ESM information transfer flag 9.9.4.5 O TV 1 */ ELEM_OPT_TV_SHORT( 0xd0 , NAS_PDU_TYPE_ESM, DE_ESM_INF_TRF_FLG , NULL ); @@ -5296,8 +5262,7 @@ nas_esm_pdn_disc_req(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo, guint3 /* Fix the lengths */ curr_len--; curr_offset++; - if (curr_len == 0) - return; + /* 27 Protocol configuration options Protocol configuration options 9.9.4.11 O TLV 3-253 */ ELEM_OPT_TLV( 0x27 , GSM_A_PDU_TYPE_GM, DE_PRO_CONF_OPT , NULL ); /* 7B Extended protocol configuration options Extended protocol configuration options 9.9.4.26 O TLV-E 4-65538 */ @@ -5318,9 +5283,6 @@ nas_esm_remote_ue_report(tvbuff_t *tvb, proto_tree *tree, packet_info *pinfo, gu curr_offset = offset; curr_len = len; - if (len == 0) - return; - pinfo->link_dir = P2P_DIR_UL; /* 79 Remote UE Context Connected Remote UE context list 9.9.4.20 O TLV-E 3-65538 */ |