diff options
author | Peter Wu <peter@lekensteyn.nl> | 2018-10-21 15:10:57 +0200 |
---|---|---|
committer | Anders Broman <a.broman58@gmail.com> | 2018-10-23 05:09:29 +0000 |
commit | 5797f602741a7505c18c2e0c505e963ca3349153 (patch) | |
tree | f700667ac74a1376eb2ff4bb5f21fb4783b9dae4 | |
parent | 24fb3a14dcf23df1e91cd0de067e97690155e4b8 (diff) |
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 <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
-rw-r--r-- | epan/dissectors/packet-lbmpdm.c | 8 |
1 files changed, 5 insertions, 3 deletions
diff --git a/epan/dissectors/packet-lbmpdm.c b/epan/dissectors/packet-lbmpdm.c index a875c8c514..6b42e506f4 100644 --- a/epan/dissectors/packet-lbmpdm.c +++ b/epan/dissectors/packet-lbmpdm.c @@ -12,6 +12,7 @@ #include "config.h" #include <epan/packet.h> +#include <epan/exceptions.h> #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]; |