From 5797f602741a7505c18c2e0c505e963ca3349153 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Sun, 21 Oct 2018 15:10:57 +0200 Subject: [PATCH] LBMPDM: fix heap-buffer-overflow (write) in dissect_segment_ofstable id_list and ofs_list contain offsets read directly from the packet. While the field type is FT_UINT32, it is somehow interpreted as signed number. This means that ofs_table->offset_list[id_list[idx]]=... could in fact result in an arbitrary write before "ofs_table->offset_list" due to id_list[idx] being negative. Another way for id_list[idx] to remain negative (-1) is for the loop to terminate before all "field_count" elements are set. Thus, remove the "datalen_remaining >= L_LBMPDM_OFFSET_ENTRY_T" check, if the offset is invalid the proto_tree_add_item accessors will throw an exception. Fixes the crash in the linked bug. Regression tested against the 8 capture files from bug 9718, its dissection results are still the same. Bug: 15132 Change-Id: If5d2f11ee47578acb80bc43ba7ed16adb27e0c02 Fixes: v1.11.3-rc1-2270-g2f4ca9c8d9 ("Initial checkin of LBM aka 29West dissectors. See Bug 9718.") Reviewed-on: https://code.wireshark.org/review/30300 Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman --- epan/dissectors/packet-lbmpdm.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/epan/dissectors/packet-lbmpdm.c b/epan/dissectors/packet-lbmpdm.c index a875c8c..6b42e50 100644 --- a/epan/dissectors/packet-lbmpdm.c +++ b/epan/dissectors/packet-lbmpdm.c @@ -12,6 +12,7 @@ #include "config.h" #include +#include #include "packet-lbm.h" /* Magic number for message header to check if data is big-endian or little-endian. */ @@ -792,7 +793,6 @@ static int dissect_segment_ofstable(tvbuff_t * tvb, int offset, packet_info * pi proto_tree * subtree = NULL; int datalen = 0; int seglen = 0; - int datalen_remaining = 0; int ofs = 0; int field_count = 0; int idx; @@ -817,9 +817,8 @@ static int dissect_segment_ofstable(tvbuff_t * tvb, int offset, packet_info * pi id_list[idx] = -1; ofs_list[idx] = -1; } - datalen_remaining = datalen; ofs = offset + L_LBMPDM_SEG_HDR_T; - for (idx = 0; (idx < field_count) && (datalen_remaining >= L_LBMPDM_OFFSET_ENTRY_T); idx++, ofs += L_LBMPDM_OFFSET_ENTRY_T) + for (idx = 0; idx < field_count; idx++, ofs += L_LBMPDM_OFFSET_ENTRY_T) { proto_item * offset_item = NULL; proto_tree * offset_tree = NULL; @@ -830,6 +829,9 @@ static int dissect_segment_ofstable(tvbuff_t * tvb, int offset, packet_info * pi id_list[idx] = (gint32)tvb_get_guint32(tvb, ofs + O_LBMPDM_OFFSET_ENTRY_T_ID, encoding); proto_tree_add_item(offset_tree, hf_lbmpdm_offset_entry_offset, tvb, ofs + O_LBMPDM_OFFSET_ENTRY_T_OFFSET, L_LBMPDM_OFFSET_ENTRY_T_OFFSET, encoding); ofs_list[idx] = (gint32)tvb_get_guint32(tvb, ofs + O_LBMPDM_OFFSET_ENTRY_T_OFFSET, encoding); + if (id_list[idx] < 0 || ofs_list[idx] < 0) { + THROW(ReportedBoundsError); + } if (id_list[idx] > max_index) { max_index = id_list[idx]; -- 1.7.12.4