diff options
author | Stig Bjørlykke <stig@bjorlykke.org> | 2017-11-20 15:15:27 +0100 |
---|---|---|
committer | Stig Bjørlykke <stig@bjorlykke.org> | 2017-11-21 07:42:17 +0000 |
commit | 4a08c63e88715656e1d6294a8978f9573e516286 (patch) | |
tree | ae600354df38246f6ac8e0bd4fc12a3b2a690078 /epan | |
parent | 90e236fd09d68bab09edb7b93227fc02de561a38 (diff) |
mqtt: Add sanity checks for MQTT v5.0 Reason Codes
Check if mqtt_msg_type is within boundaries of hf_rcode and gives
a valid hfindex.
Change-Id: Ib8ea710d7cd6c61ec493e218d64b50f6faa720c4
Reviewed-on: https://code.wireshark.org/review/24509
Petri-Dish: Stig Bjørlykke <stig@bjorlykke.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Stig Bjørlykke <stig@bjorlykke.org>
Diffstat (limited to 'epan')
-rw-r--r-- | epan/dissectors/packet-mqtt.c | 87 |
1 files changed, 53 insertions, 34 deletions
diff --git a/epan/dissectors/packet-mqtt.c b/epan/dissectors/packet-mqtt.c index 79f6b26ad7..a441066808 100644 --- a/epan/dissectors/packet-mqtt.c +++ b/epan/dissectors/packet-mqtt.c @@ -714,6 +714,38 @@ static guint dissect_string(tvbuff_t *tvb, proto_tree *tree, guint offset, int h return 2 + prop_len; } +/* MQTT v5.0: Reason Codes */ +static void dissect_mqtt_reason_code(proto_tree *mqtt_tree, tvbuff_t *tvb, guint offset, guint8 mqtt_msg_type) +{ + static const int *hf_rcode[] = { + NULL, /* RESERVED */ + NULL, /* CONNECT */ + &hf_mqtt_reason_code_connack, + NULL, /* PUBLISH */ + &hf_mqtt_reason_code_puback, + &hf_mqtt_reason_code_pubrec, + &hf_mqtt_reason_code_pubrel, + &hf_mqtt_reason_code_pubcomp, + NULL, /* SUBSCRIBE */ + &hf_mqtt_reason_code_suback, + NULL, /* UNSUBSCRIBE */ + &hf_mqtt_reason_code_unsuback, + NULL, /* PINGREQ */ + NULL, /* PINGRESP */ + &hf_mqtt_reason_code_disconnect, + &hf_mqtt_reason_code_auth + }; + + if (mqtt_msg_type < (sizeof hf_rcode / sizeof hf_rcode[0])) + { + const int *hfindex = hf_rcode[mqtt_msg_type]; + if (hfindex) + { + proto_tree_add_item(mqtt_tree, *hfindex, tvb, offset, 1, ENC_BIG_ENDIAN); + } + } +} + /* MQTT v5.0: dissect the MQTT properties */ static guint dissect_mqtt_properties(tvbuff_t *tvb, proto_tree *mqtt_tree, guint offset) { @@ -825,7 +857,6 @@ static int dissect_mqtt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi conversation_t *conv; mqtt_conv_t *mqtt; guint offset = 0; - int hf_selector; static const int *publish_fields[] = { &hf_mqtt_msg_type, @@ -875,28 +906,6 @@ static int dissect_mqtt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi NULL }; - /* For v3.1/v3.1.1 versions, column 0 is "Return Code" - * For v5.0, column 1 is "Reason Code" - */ - static const int *hf_rcode[MQTT_RESERVED_16][2] = { - { NULL, NULL }, - { NULL, NULL }, /* CONNECT */ - { &hf_mqtt_conack_code, &hf_mqtt_reason_code_connack }, - { NULL, NULL }, - { NULL, &hf_mqtt_reason_code_puback }, - { NULL, &hf_mqtt_reason_code_pubrec }, - { NULL, &hf_mqtt_reason_code_pubrel }, - { NULL, &hf_mqtt_reason_code_pubcomp }, - { NULL, NULL }, - { &hf_mqtt_suback_qos, &hf_mqtt_reason_code_suback }, - { NULL, NULL }, - { NULL, &hf_mqtt_reason_code_unsuback }, - { NULL, NULL }, - { NULL, NULL }, - { NULL, &hf_mqtt_reason_code_disconnect }, - { NULL, &hf_mqtt_reason_code_auth }, - }; - /* Extract the message ID */ mqtt_fixed_hdr = tvb_get_guint8(tvb, offset); mqtt_msg_type = mqtt_fixed_hdr >> 4; @@ -946,12 +955,6 @@ static int dissect_mqtt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi proto_tree_add_uint64(mqtt_tree, hf_mqtt_msg_len, tvb, offset, mqtt_len_offset, msg_len); offset += mqtt_len_offset; - /* mqtt->runtime_proto_version is only modified by CONNECT. - * CONNECT does not require hf_selector, so we assign it - * here to be used later by CONNACK and SUBACK. - */ - hf_selector = (mqtt->runtime_proto_version == MQTT_PROTO_V50 ? 1 : 0); - switch (mqtt_msg_type) { case MQTT_CONNECT: @@ -1034,7 +1037,15 @@ static int dissect_mqtt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi } offset += 1; - proto_tree_add_item(mqtt_tree, *hf_rcode[MQTT_CONNACK][hf_selector], tvb, offset, 1, ENC_BIG_ENDIAN); + if ((mqtt->runtime_proto_version == MQTT_PROTO_V31) || + (mqtt->runtime_proto_version == MQTT_PROTO_V311)) + { + proto_tree_add_item(mqtt_tree, hf_mqtt_conack_code, tvb, offset, 1, ENC_BIG_ENDIAN); + } + else + { + dissect_mqtt_reason_code(mqtt_tree, tvb, offset, mqtt_msg_type); + } offset += 1; if (mqtt->runtime_proto_version == MQTT_PROTO_V50) @@ -1140,7 +1151,15 @@ static int dissect_mqtt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi while (offset < tvb_reported_length(tvb)) { - proto_tree_add_item(mqtt_tree, *hf_rcode[MQTT_SUBACK][hf_selector], tvb, offset, 1, ENC_BIG_ENDIAN); + if ((mqtt->runtime_proto_version == MQTT_PROTO_V31) || + (mqtt->runtime_proto_version == MQTT_PROTO_V311)) + { + proto_tree_add_item(mqtt_tree, hf_mqtt_suback_qos, tvb, offset, 1, ENC_BIG_ENDIAN); + } + else + { + dissect_mqtt_reason_code(mqtt_tree, tvb, offset, mqtt_msg_type); + } offset += 1; } break; @@ -1154,7 +1173,7 @@ static int dissect_mqtt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi if (mqtt->runtime_proto_version == MQTT_PROTO_V50) { - proto_tree_add_item(mqtt_tree, *hf_rcode[mqtt_msg_type][1], tvb, offset, 1, ENC_BIG_ENDIAN); + dissect_mqtt_reason_code(mqtt_tree, tvb, offset, mqtt_msg_type); offset += 1; offset += dissect_mqtt_properties(tvb, mqtt_tree, offset); @@ -1171,7 +1190,7 @@ static int dissect_mqtt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi while (offset < tvb_reported_length(tvb)) { - proto_tree_add_item(mqtt_tree, *hf_rcode[MQTT_UNSUBACK][1], tvb, offset, 1, ENC_BIG_ENDIAN); + dissect_mqtt_reason_code(mqtt_tree, tvb, offset, mqtt_msg_type); offset += 1; } } @@ -1195,7 +1214,7 @@ static int dissect_mqtt(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi */ if (mqtt->runtime_proto_version == MQTT_PROTO_V50 && mqtt_msg_len > 0) { - proto_tree_add_item(mqtt_tree, *hf_rcode[mqtt_msg_type][1], tvb, offset, 1, ENC_BIG_ENDIAN); + dissect_mqtt_reason_code(mqtt_tree, tvb, offset, mqtt_msg_type); offset += 1; /* 3.14.2.2 DISCONNECT Properties: |