diff options
author | Pascal Quantin <pascal.quantin@gmail.com> | 2018-02-17 22:51:39 +0100 |
---|---|---|
committer | Michael Mann <mmann78@netscape.net> | 2018-02-18 02:21:24 +0000 |
commit | 5d99febe66e96b55a1defa58a906be254bad3a51 (patch) | |
tree | 35bafc2673afd71ecf7b3e7caaff931b06d248b8 /epan/dissectors/packet-s7comm.c | |
parent | cae52d27d646c04734726b54edf9aacddc3153e8 (diff) |
S7comm: fix range check to prevent infinite loop when upper bound is 255
While we are at it, fix identification of not last element in a few
places
Bug: 14423
Change-Id: I568530949d09ddfd8c5c58d24050dfed32ce10f5
Reviewed-on: https://code.wireshark.org/review/25851
Reviewed-by: Pascal Quantin <pascal.quantin@gmail.com>
Petri-Dish: Pascal Quantin <pascal.quantin@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Michael Mann <mmann78@netscape.net>
Diffstat (limited to 'epan/dissectors/packet-s7comm.c')
-rw-r--r-- | epan/dissectors/packet-s7comm.c | 22 |
1 files changed, 11 insertions, 11 deletions
diff --git a/epan/dissectors/packet-s7comm.c b/epan/dissectors/packet-s7comm.c index 2863127b81..ba82ab8d33 100644 --- a/epan/dissectors/packet-s7comm.c +++ b/epan/dissectors/packet-s7comm.c @@ -2476,7 +2476,7 @@ s7comm_syntaxid_dbread(tvbuff_t *tvb, proto_tree_add_item_ret_uint(tree, hf_s7comm_item_dbread_numareas, tvb, offset, 1, ENC_BIG_ENDIAN, &number_of_areas); proto_item_append_text(tree, " (%d Data-Areas of Syntax-Id DBREAD)", number_of_areas); offset += 1; - for (i = 1; i <= number_of_areas; i++) { + for (i = 0; i < number_of_areas; i++) { sub_item = proto_tree_add_item(tree, hf_s7comm_param_subitem, tvb, offset, 5, ENC_NA); sub_item_tree = proto_item_add_subtree(sub_item, ett_s7comm_param_subitem); proto_tree_add_item_ret_uint(sub_item_tree, hf_s7comm_item_dbread_length, tvb, offset, 1, ENC_BIG_ENDIAN, &len); @@ -2486,7 +2486,7 @@ s7comm_syntaxid_dbread(tvbuff_t *tvb, proto_tree_add_item_ret_uint(sub_item_tree, hf_s7comm_item_dbread_startadr, tvb, offset, 2, ENC_BIG_ENDIAN, &bytepos); offset += 2; /* Display in pseudo S7-Any Format */ - proto_item_append_text(sub_item, " [%d]: (DB%d.DBB %d BYTE %d)", i, db, bytepos, len); + proto_item_append_text(sub_item, " [%d]: (DB%d.DBB %d BYTE %d)", i+1, db, bytepos, len); } return offset; } @@ -2714,12 +2714,12 @@ s7comm_decode_response_write_data(tvbuff_t *tvb, proto_item *item = NULL; proto_tree *item_tree = NULL; - for (i = 1; i <= item_count; i++) { + for (i = 0; i < item_count; i++) { ret_val = tvb_get_guint8(tvb, offset); /* Insert a new tree for every item */ item = proto_tree_add_item(tree, hf_s7comm_data_item, tvb, offset, 1, ENC_NA); item_tree = proto_item_add_subtree(item, ett_s7comm_data_item); - proto_item_append_text(item, " [%d]: (%s)", i, val_to_str(ret_val, s7comm_item_return_valuenames, "Unknown code: 0x%02x")); + proto_item_append_text(item, " [%d]: (%s)", i+1, val_to_str(ret_val, s7comm_item_return_valuenames, "Unknown code: 0x%02x")); proto_tree_add_uint(item_tree, hf_s7comm_data_returncode, tvb, offset, 1, ret_val); offset += 1; } @@ -2759,7 +2759,7 @@ s7comm_decode_response_read_data(tvbuff_t *tvb, } } else { /* Standard */ - for (i = 1; i <= item_count; i++) { + for (i = 0; i < item_count; i++) { ret_val = tvb_get_guint8(tvb, offset); if (ret_val == S7COMM_ITEM_RETVAL_RESERVED || ret_val == S7COMM_ITEM_RETVAL_DATA_OK || @@ -2781,7 +2781,7 @@ s7comm_decode_response_read_data(tvbuff_t *tvb, } /* the PLC places extra bytes at the end of all but last result, if length is not a multiple of 2 */ - if ((len % 2) && (i < item_count)) { + if ((len % 2) && (i < (item_count-1))) { len2 = len + 1; } else { len2 = len; @@ -2790,7 +2790,7 @@ s7comm_decode_response_read_data(tvbuff_t *tvb, /* Insert a new tree for every item */ item = proto_tree_add_item(tree, hf_s7comm_data_item, tvb, offset, len + head_len, ENC_NA); item_tree = proto_item_add_subtree(item, ett_s7comm_data_item); - proto_item_append_text(item, " [%d]: (%s)", i, val_to_str(ret_val, s7comm_item_return_valuenames, "Unknown code: 0x%02x")); + proto_item_append_text(item, " [%d]: (%s)", i+1, val_to_str(ret_val, s7comm_item_return_valuenames, "Unknown code: 0x%02x")); proto_tree_add_uint(item_tree, hf_s7comm_data_returncode, tvb, offset, 1, ret_val); proto_tree_add_uint(item_tree, hf_s7comm_data_transport_size, tvb, offset + 1, 1, tsize); @@ -4004,11 +4004,11 @@ s7comm_decode_ud_cpu_alarm_main(tvbuff_t *tvb, nr_objects = tvb_get_guint8(tvb, offset); proto_tree_add_uint(msg_item_tree, hf_s7comm_cpu_alarm_message_nr_objects, tvb, offset, 1, nr_objects); offset += 1; - for (i = 1; i <= nr_objects; i++) { + for (i = 0; i < nr_objects; i++) { msg_obj_start_offset = offset; msg_obj_item = proto_tree_add_item(msg_item_tree, hf_s7comm_cpu_alarm_message_obj_item, tvb, offset, 0, ENC_NA); msg_obj_item_tree = proto_item_add_subtree(msg_obj_item, ett_s7comm_cpu_alarm_message_object); - proto_item_append_text(msg_obj_item_tree, " [%d]", i); + proto_item_append_text(msg_obj_item_tree, " [%d]", i+1); if (type == S7COMM_UD_TYPE_REQ || type == S7COMM_UD_TYPE_PUSH) { proto_tree_add_item(msg_obj_item_tree, hf_s7comm_item_varspec, tvb, offset, 1, ENC_BIG_ENDIAN); offset += 1; @@ -4635,7 +4635,7 @@ s7comm_decode_ud_cyclic_subfunc(tvbuff_t *tvb, offset = s7comm_decode_param_item(tvb, offset, data_tree, i); /* if length is not a multiple of 2 and this is not the last item, then add a fill-byte */ len_item = offset - offset_old; - if ((len_item % 2) && (i < item_count)) { + if ((len_item % 2) && (i < (item_count-1))) { offset += 1; } } @@ -5036,7 +5036,7 @@ s7comm_decode_req_resp(tvbuff_t *tvb, offset = s7comm_decode_param_item(tvb, offset, param_tree, i); /* if length is not a multiple of 2 and this is not the last item, then add a fill-byte */ len = offset - offset_old; - if ((len % 2) && (i < item_count)) { + if ((len % 2) && (i < (item_count-1))) { offset += 1; } } |