diff options
author | Stig Bjørlykke <stig@bjorlykke.org> | 2018-02-22 22:01:20 +0100 |
---|---|---|
committer | Gerald Combs <gerald@wireshark.org> | 2018-02-22 22:25:25 +0000 |
commit | f14f76d12a3107f0a89884f084640925ba033b28 (patch) | |
tree | 1b9aa5712d703988d3c590947b5e97e2cbb2419d | |
parent | 380381ce72e00db3e845eb6507c55b61739d783a (diff) |
dmp: Allow multiple SecurityCategories again
A Security Classification in DMP may have multiple Security Categories
so don't restrict this to only one. Add a arbitrary limit of 255 to
avoid a long dissector loop in malformed packets.
This fixes a bug introduced in g85bbda51.
Bug: 14408
Change-Id: I48e7a61a097c58dfcf21e9c9ed3147cf1573dae6
Reviewed-on: https://code.wireshark.org/review/26011
Petri-Dish: Stig Bjørlykke <stig@bjorlykke.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Gerald Combs <gerald@wireshark.org>
-rw-r--r-- | epan/dissectors/packet-dmp.c | 36 |
1 files changed, 26 insertions, 10 deletions
diff --git a/epan/dissectors/packet-dmp.c b/epan/dissectors/packet-dmp.c index e41659027f..e59ff4c32b 100644 --- a/epan/dissectors/packet-dmp.c +++ b/epan/dissectors/packet-dmp.c @@ -478,6 +478,7 @@ static expert_field ei_analysis_ack_dup_no = EI_INIT; static expert_field ei_analysis_ack_unexpected = EI_INIT; static expert_field ei_analysis_msg_missing = EI_INIT; static expert_field ei_analysis_retrans_no = EI_INIT; +static expert_field ei_too_many_sec_cat = EI_INIT; static dissector_handle_t dmp_handle; @@ -3423,7 +3424,7 @@ static gint dissect_dmp_notification (tvbuff_t *tvb, packet_info *pinfo _U_, static gint dissect_dmp_security_category (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, const gchar **label_string, - gint offset, guint8 ext, gboolean extended) + gint offset, guint8 *ext) { proto_tree *field_tree = NULL; proto_item *tf = NULL, *tr = NULL; @@ -3436,7 +3437,7 @@ static gint dissect_dmp_security_category (tvbuff_t *tvb, packet_info *pinfo, offset, 1, message, "Security Categories"); field_tree = proto_item_add_subtree (tf, ett_message_sec_cat); - switch (ext) { + switch (*ext) { case SEC_CAT_EXT_NONE: proto_tree_add_item (field_tree, hf_message_sec_cat_cl, tvb, offset, 1, ENC_BIG_ENDIAN); @@ -3490,6 +3491,7 @@ static gint dissect_dmp_security_category (tvbuff_t *tvb, packet_info *pinfo, } proto_item_append_text (tf, " (0x%2.2x)", message); + *ext = 0; /* Reset extended bits */ if (dmp.version == 1) { tr = proto_tree_add_item (field_tree, hf_reserved_0x02, tvb, offset, 1, ENC_BIG_ENDIAN); @@ -3500,20 +3502,22 @@ static gint dissect_dmp_security_category (tvbuff_t *tvb, packet_info *pinfo, if (message & 0x01) { expert_add_info(pinfo, tr, &ei_reserved_value); } + offset += 1; } else { tr = proto_tree_add_item (field_tree, hf_message_sec_cat_extended, tvb, offset, 1, ENC_BIG_ENDIAN); if ((message & 0x01) && (message & 0x02)) { expert_add_info(pinfo, tr, &ei_reserved_value); - } else if ((message & 0x01 || message & 0x02) && !extended) { + } else if (message & 0x01 || message & 0x02) { proto_item_append_text (tf, " (extended)"); - offset = dissect_dmp_security_category (tvb, pinfo, tree, label_string, offset+1, message & 0x03, TRUE); + *ext = message & 0x03; } + offset += 1; if (country_code) { - proto_tree_add_item (field_tree, hf_message_sec_cat_country_code, tvb, offset+1, 1, ENC_BIG_ENDIAN); - proto_item_append_text (tf, " (rel-to country-code: %d)", tvb_get_guint8 (tvb, offset+1)); + proto_tree_add_item (field_tree, hf_message_sec_cat_country_code, tvb, offset, 1, ENC_BIG_ENDIAN); + proto_item_append_text (tf, " (rel-to country-code: %d)", tvb_get_guint8 (tvb, offset)); proto_item_set_len (tf, 2); - offset++; + offset += 1; } } @@ -3723,10 +3727,19 @@ static gint dissect_dmp_content (tvbuff_t *tvb, packet_info *pinfo, /* Security Categories */ if (dmp_sec_pol == NATO || dmp_sec_pol == NATIONAL || dmp_sec_pol == EXTENDED_NATIONAL) { - offset = dissect_dmp_security_category (tvb, pinfo, message_tree, &label_string, offset, 0, FALSE); + guint8 ext = 0; + guint sec_cat_count = 0; + do { + offset = dissect_dmp_security_category (tvb, pinfo, message_tree, &label_string, offset, &ext); + sec_cat_count++; + } while (ext != 0 && sec_cat_count < G_MAXUINT8); + if (sec_cat_count == G_MAXUINT8) { + /* This is a arbitrary limit to avoid a long dissector loop. */ + expert_add_info(pinfo, en, &ei_too_many_sec_cat); + } proto_item_append_text (en, ", Security Label: %s", label_string); tf = proto_tree_add_string (message_tree, hf_message_sec_label, tvb, loffset, - offset - loffset + 1, label_string); + offset - loffset, label_string); PROTO_ITEM_SET_GENERATED (tf); } else { tf = proto_tree_add_item (message_tree, hf_message_sec_cat_val, tvb, offset, 1, ENC_BIG_ENDIAN); @@ -3740,8 +3753,8 @@ static gint dissect_dmp_content (tvbuff_t *tvb, packet_info *pinfo, proto_tree_add_item (field_tree, hf_message_sec_cat_bit2, tvb, offset, 1, ENC_BIG_ENDIAN); proto_tree_add_item (field_tree, hf_message_sec_cat_bit1, tvb, offset, 1, ENC_BIG_ENDIAN); proto_tree_add_item (field_tree, hf_message_sec_cat_bit0, tvb, offset, 1, ENC_BIG_ENDIAN); + offset += 1; } - offset += 1; if (dmp.msg_type == STANAG || dmp.msg_type == IPM) { /* Expiry Time */ @@ -4877,6 +4890,9 @@ void proto_register_dmp (void) { &ei_checksum_bad, { "dmp.checksum_bad.expert", PI_CHECKSUM, PI_WARN, "Bad checksum", EXPFILL } }, + { &ei_too_many_sec_cat, + { "dmp.too_many_security_categories", PI_PROTOCOL, PI_ERROR, + "Too many security categories", EXPFILL } }, }; static uat_field_t attributes_flds[] = { |