s7comm dissector infinite loop error is fixed.

When list_count and list_len are large numbers, their
multiplication exceeds guint16. The multiplication is casted to
guint32. list_len was controlled whether it is 0 or not. However,
list_count should also have been controlled. The control for
list_count is also added to the code. Moreover, if their
multiplication is equal to zero, it should be checked before or in
for loop since it causes infinite loop and it adds more than expected
szl data trees.

Change-Id: I4bb2c076ef830581e529fea05a1d9175feab171c
Reviewed-on: https://code.wireshark.org/review/29979
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
This commit is contained in:
basakkal 2018-10-02 04:59:23 -07:00 committed by Anders Broman
parent 925f8119d7
commit e885798f22
1 changed files with 7 additions and 3 deletions

View File

@ -3998,8 +3998,8 @@ s7comm_decode_ud_cpu_szl_subfunc(tvbuff_t *tvb,
* it's not possible to decode this and following telegrams without knowing the previous requests.
*/
tbytes = 0;
if (list_len > 0) {
if ((list_count * list_len) > (len - 8)) {
if (list_len > 0 && list_count > 0) {
if ( (guint32) (list_count * list_len) > (guint32) (len - 8)) {
list_count = (len - 8) / list_len;
/* remind the number of trailing bytes */
if (list_count > 0) {
@ -4007,10 +4007,14 @@ s7comm_decode_ud_cpu_szl_subfunc(tvbuff_t *tvb,
}
}
}
else {
tbytes = len - 8;
}
offset += 2;
/* Add a Data element for each partlist */
if (len > 8) { /* minimum length of a correct szl data part is 8 bytes */
for (i = 1; i <= list_count; i++) {
for (i = 1; i <= list_count && (list_count * list_len != 0); i++) {
/* Add a separate tree for the SZL data */
szl_item = proto_tree_add_item(data_tree, hf_s7comm_userdata_szl_tree, tvb, offset, list_len, ENC_NA);
szl_item_tree = proto_item_add_subtree(szl_item, ett_s7comm_szl);