DOCSIS: fix null-pointer crash in OPT-RSP TLVs decoding

"tmp_fh->next" is NULL on the initial visit and thus
"tvb_get_guint8(tmp_fh->tvb_data,1)" crashes.

It is not entirely clear to me how reassembly should work in this DOCSIS
message, but based on the description in "6.4.45 OFDM Downstream Profile
Test Response (OPT-RSP)" in the DOCSIS MAC and Upper Layer Protocols
Interface Specification (CM-SP-MULPIv3.1-I07-150910), I suppose that it
was trying to support decoding of (a sequence of) TLVs where the value
is too large for a single frame.

Bug: 14954
Change-Id: I2eec91d0ca6356b2af61bfe55381c300c8872039
Fixes: v2.9.0rc0-1171-g738818fe4d ("DOCSIS: Added decoding for OPT (OFDM Downstream Profile Test) messages")
Reviewed-on: https://code.wireshark.org/review/28712
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

1 files changed, 10 insertions, 14 deletions

diff --git a/epan/dissectors/packet-docsis-macmgmt.c b/epan/dissectors/packet-docsis-macmgmt.c
index 332777ebb8..8cb2afaaf2 100644
--- a/epan/dissectors/packet-docsis-macmgmt.c
+++ b/epan/dissectors/packet-docsis-macmgmt.c
@@ -6418,23 +6418,19 @@ dissect_optrsp (tvbuff_t * tvb, packet_info * pinfo, proto_tree * tree, void* da
     fragment_head *tmp_fh = fragment_get(&docsis_opt_tlv_reassembly_table, pinfo, downstream_channel_id, NULL);
 
     /*Calculating offset*/
-    guint offset =0;
-    guint tlv_length = 0;
-    if (tmp_fh) {
-      tmp_fh = tmp_fh->next;
-      tlv_length  = 256*tvb_get_guint8(tmp_fh->tvb_data,1) + tvb_get_guint8(tmp_fh->tvb_data,2);
-
-      while(tmp_fh) {
-        offset += tmp_fh->len;
-        if (offset > tlv_length + 3) {
-          /*Updating tlv_length with next top TLV*/
-          tlv_length += 256*tvb_get_guint8(tmp_fh->tvb_data,offset-tlv_length - 3) + tvb_get_guint8(tmp_fh->tvb_data,offset-tlv_length - 3);
-        }
-        tmp_fh = tmp_fh->next;
+    guint offset = 0;
+    guint tlvs_length = 0;
+    while (tmp_fh) {
+      offset += tmp_fh->len;
+      while (tlvs_length < offset) {
+        /* Assumes that the Type and Lengths fields are always present in a
+         * single buffer (the value can still be fragmented). */
+        tlvs_length += 3 + tvb_get_ntohs(tmp_fh->tvb_data, tlvs_length + 1);
       }
+      tmp_fh = tmp_fh->next;
     }
 
-    if (offset == tlv_length +3) {
+    if (offset == tlvs_length) {
       /* Save address pointers. */
       copy_address_shallow(&save_src, &pinfo->src);
       copy_address_shallow(&save_dst, &pinfo->dst);