osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

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>
This commit is contained in:
Pascal Quantin 2017-01-27 18:20:40 +01:00 committed by Alexis La Goutte
parent 275594231d
commit 5dde07c8fd
4 changed files with 8 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
epan/dissectors/packet-gsm_a_common.h
View File

@ -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; \
}
/*

10
epan/dissectors/packet-gsm_a_dtap.c
View File

@ -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);

2
epan/dissectors/packet-gsm_a_rr.c
View File

@ -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);

42
epan/dissectors/packet-nas_eps.c
View File

@ -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 */