FP - Checks to avoid NULL dereferencing

Making sure channel specific structs are not null

Change-Id: I9f979db4e24bb7bd490cf29a5c68ef32d1b029db
Reviewed-on: https://code.wireshark.org/review/21754
Petri-Dish: Pascal Quantin <pascal.quantin@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Pascal Quantin <pascal.quantin@gmail.com>

1 files changed, 70 insertions, 38 deletions

diff --git a/epan/dissectors/packet-umts_fp.c b/epan/dissectors/packet-umts_fp.c
index a2c0d0746f..830da88eac 100644
--- a/epan/dissectors/packet-umts_fp.c
+++ b/epan/dissectors/packet-umts_fp.c
@@ -238,6 +238,7 @@ static expert_field ei_fp_bad_header_checksum = EI_INIT;
 static expert_field ei_fp_crci_error_bit_set_for_tb = EI_INIT;
 static expert_field ei_fp_spare_extension = EI_INIT;
 static expert_field ei_fp_no_per_frame_info = EI_INIT;
+static expert_field ei_fp_no_per_conv_channel_info = EI_INIT;
 
 static dissector_handle_t rlc_bcch_handle;
 static dissector_handle_t mac_fdd_dch_handle;
@@ -4920,17 +4921,19 @@ fp_set_per_packet_inf_from_conv(conversation_t *p_conv,
     int       offset = 0, i=0, j=0, num_tbs, chan, tb_size, tb_bit_off;
     gboolean  is_control_frame;
     gboolean  is_known_dcch_tf,is_special_case_dch_24;
-    umts_mac_info *macinf;
-    rlc_info *rlcinf;
+    umts_mac_info *macinf = NULL;
+    rlc_info *rlcinf = NULL;
     guint8 fake_lchid=0;
     gint *cur_val=NULL;
     guint32 user_identity;
     fp_hsdsch_channel_info_t* fp_hsdsch_channel_info = NULL;
     fp_edch_channel_info_t* fp_edch_channel_info = NULL;
     fp_pch_channel_info_t *fp_pch_channel_info = NULL;
+    fp_fach_channel_info_t* fp_fach_channel_info = NULL;
+    fp_rach_channel_info_t* fp_rach_channel_info = NULL;
+    gboolean info_missing = FALSE;
 
     fpi = wmem_new0(wmem_file_scope(), fp_info);
-    p_add_proto_data(wmem_file_scope(), pinfo, proto_fp, 0, fpi);
 
     fpi->iface_type = p_conv_data->iface_type;
     fpi->division = p_conv_data->division;
@@ -4967,14 +4970,18 @@ fp_set_per_packet_inf_from_conv(conversation_t *p_conv,
     switch (fpi->channel) {
         case CHANNEL_HSDSCH: /* HS-DSCH - High Speed Downlink Shared Channel */
             fp_hsdsch_channel_info = (fp_hsdsch_channel_info_t*)p_conv_data->channel_specific_info;
-            fpi->hsdsch_entity = fp_hsdsch_channel_info->hsdsch_entity;
+            if(fp_hsdsch_channel_info == NULL) {
+                proto_tree_add_expert_format(tree, pinfo, &ei_fp_no_per_conv_channel_info, tvb, offset, -1,
+                                      "Can't dissect HS-DSCH FP stream because no per-conversation channel info was attached!");
+                info_missing = TRUE;
+                break;
+            }
+            rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
             macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
+            fpi->hsdsch_entity = fp_hsdsch_channel_info->hsdsch_entity;
             fpi->hsdsch_macflowd_id = fp_hsdsch_channel_info->hsdsch_macdflow_id;
             macinf->content[0] = hsdsch_macdflow_id_mac_content_map[fp_hsdsch_channel_info->hsdsch_macdflow_id]; /*MAC_CONTENT_PS_DTCH;*/
             macinf->lchid[0] = fp_hsdsch_channel_info->hsdsch_macdflow_id;
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_umts_mac, 0, macinf);
-
-            rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
 
             /*Figure out RLC_MODE based on MACd-flow-ID, basically MACd-flow-ID = 0 then it's SRB0 == UM else AM*/
             rlcinf->mode[0] = hsdsch_macdflow_id_rlc_map[fp_hsdsch_channel_info->hsdsch_macdflow_id];
@@ -5015,16 +5022,20 @@ fp_set_per_packet_inf_from_conv(conversation_t *p_conv,
             rlcinf->li_size[0] = RLC_LI_7BITS;
             rlcinf->ciphered[0] = FALSE;
             rlcinf->deciphered[0] = FALSE;
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_rlc, 0, rlcinf);
-
 
             return fpi;
 
         case CHANNEL_EDCH:
             fp_edch_channel_info = (fp_edch_channel_info_t*)p_conv_data->channel_specific_info;
-            /*Most configuration is now done in the actual dissecting function*/
-            macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
+            if(fp_edch_channel_info == NULL) {
+                proto_tree_add_expert_format(tree, pinfo, &ei_fp_no_per_conv_channel_info, tvb, offset, -1,
+                                      "Can't dissect E-DCH FP stream because no per-conversation channel info was attached!");
+                info_missing = TRUE;
+                break;
+            }
             rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
+            macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
+            /*Most configuration is now done in the actual dissecting function*/
             fpi->no_ddi_entries = fp_edch_channel_info->no_ddi_entries;
             for (i=0; i<fpi->no_ddi_entries; i++) {
                 fpi->edch_ddi[i] = fp_edch_channel_info->edch_ddi[i];    /*Set the DDI value*/
@@ -5036,24 +5047,23 @@ fp_set_per_packet_inf_from_conv(conversation_t *p_conv,
             }
             fpi->edch_type = fp_edch_channel_info->edch_type;
 
-           /* macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
-            macinf->content[0] = MAC_CONTENT_PS_DTCH;*/
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_umts_mac, 0, macinf);
-
-
             /* For RLC re-assembly to work we need a urnti signaled from NBAP */
             rlcinf->urnti[0] = fpi->com_context_id;
-           /* rlcinf->mode[0] = RLC_AM;*/
+            /* rlcinf->mode[0] = RLC_AM;*/
             rlcinf->li_size[0] = RLC_LI_7BITS;
             rlcinf->ciphered[0] = FALSE;
             rlcinf->deciphered[0] = FALSE;
 
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_rlc, 0, rlcinf);
-
             return fpi;
 
         case CHANNEL_PCH:
             fp_pch_channel_info = (fp_pch_channel_info_t*)p_conv_data->channel_specific_info;
+            if(fp_pch_channel_info == NULL) {
+                proto_tree_add_expert_format(tree, pinfo, &ei_fp_no_per_conv_channel_info, tvb, offset, -1,
+                                      "Can't dissect PCH FP stream because no per-conversation channel info was attached!");
+                info_missing = TRUE;
+                break;
+            }
             fpi->paging_indications = fp_pch_channel_info->paging_indications;
             fpi->num_chans = p_conv_data->num_dch_in_flow;
 
@@ -5074,9 +5084,9 @@ fp_set_per_packet_inf_from_conv(conversation_t *p_conv,
                 /* control frame, we're done */
                 return fpi;
             }
-
             rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
             macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
+
             offset = 2; /* To correctly read the TFI */
             fakes  = 5; /* Reset fake counter */
             for (chan=0; chan < fpi->num_chans; chan++) { /* Iterate over the DCH channels in the flow (each given a TFI) */
@@ -5164,30 +5174,35 @@ fp_set_per_packet_inf_from_conv(conversation_t *p_conv,
 
                 offset++;
             }
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_umts_mac, 0, macinf);
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_rlc, 0, rlcinf);
             /* Set offset to point to first TFI
              * the Number of TFI's = number of DCH's in the flow
              */
             offset = 2;
             break;
         case CHANNEL_FACH_FDD:
+            fp_fach_channel_info = (fp_fach_channel_info_t*)p_conv_data->channel_specific_info;
+            if(fp_fach_channel_info == NULL) {
+                proto_tree_add_expert_format(tree, pinfo, &ei_fp_no_per_conv_channel_info, tvb, offset, -1,
+                                      "Can't dissect FACH FP stream because no per-conversation channel info was attached!");
+                info_missing = TRUE;
+                break;
+            }
             fpi->num_chans = p_conv_data->num_dch_in_flow;
             if (is_control_frame) {
                 /* control frame, we're done */
                 return fpi;
             }
+            rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
+            macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
             /* Set offset to point to first TFI
              * the Number of TFI's = number of DCH's in the flow
              */
             offset = 2;
             /* Set MAC data */
-            macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
             macinf->ctmux[0]   = 1;
             macinf->content[0] = MAC_CONTENT_DCCH;
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_umts_mac, 0, macinf);
+
             /* Set RLC data */
-            rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
             /* Make configurable ?(avaliable in NBAP?) */
             /* For RLC re-assembly to work we need to fake urnti: using the conversation's ID and the prefix of 0xFFF */
             rlcinf->urnti[0] = (p_conv->conv_index | 0xFFF00000);
@@ -5196,42 +5211,58 @@ fp_set_per_packet_inf_from_conv(conversation_t *p_conv,
             rlcinf->li_size[0] = RLC_LI_7BITS;
             rlcinf->ciphered[0] = FALSE;
             rlcinf->deciphered[0] = FALSE;
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_rlc, 0, rlcinf);
             break;
 
         case CHANNEL_RACH_FDD:
+            fp_rach_channel_info = (fp_rach_channel_info_t*)p_conv_data->channel_specific_info;
+            if(fp_rach_channel_info == NULL) {
+                proto_tree_add_expert_format(tree, pinfo, &ei_fp_no_per_conv_channel_info, tvb, offset, -1,
+                                      "Can't dissect RACH FP stream because no per-conversation channel info was attached!");
+                info_missing = TRUE;
+                break;
+            }
             fpi->num_chans = p_conv_data->num_dch_in_flow;
             if (is_control_frame) {
                 /* control frame, we're done */
                 return fpi;
             }
+            rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
+            macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
             /* Set offset to point to first TFI
              * the Number of TFI's = number of DCH's in the flow
              */
             offset = 2;
             /* set MAC data */
-            macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
-            rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
             for ( chan = 0; chan < fpi->num_chans; chan++ ) {
                     macinf->ctmux[chan]   = 1;
                     macinf->content[chan] = MAC_CONTENT_DCCH;
                     rlcinf->urnti[chan] = fpi->com_context_id;    /*Note that MAC probably will change this*/
             }
-
-
-
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_umts_mac, 0, macinf);
-            p_add_proto_data(wmem_file_scope(), pinfo, proto_rlc, 0, rlcinf);
             break;
         case CHANNEL_HSDSCH_COMMON:
-                rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
-                macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
-                p_add_proto_data(wmem_file_scope(), pinfo, proto_umts_mac, 0, macinf);
-                p_add_proto_data(wmem_file_scope(), pinfo, proto_rlc, 0, rlcinf);
+            rlcinf = wmem_new0(wmem_file_scope(), rlc_info);
+            macinf = wmem_new0(wmem_file_scope(), umts_mac_info);
             break;
         default:
             expert_add_info(pinfo, NULL, &ei_fp_transport_channel_type_unknown);
-            return NULL;
+            info_missing = TRUE;
+            break;
+    }
+
+    if(info_missing) {
+        /* Some information was missing in the conversation struct and the FP info isn't complete */
+        wmem_free(wmem_file_scope(),fpi);
+        return NULL;
+    }
+
+    // Attach FP info
+    p_add_proto_data(wmem_file_scope(), pinfo, proto_fp, 0, fpi);
+    // Attach MAC & RLC info only if created
+    if(macinf) {
+        p_add_proto_data(wmem_file_scope(), pinfo, proto_umts_mac, 0, macinf);
+    }
+    if(rlcinf) {
+        p_add_proto_data(wmem_file_scope(), pinfo, proto_rlc, 0, rlcinf);
     }
 
     /* Peek at the packet as the per packet info seems not to take the tfi into account */
@@ -6655,6 +6686,7 @@ void proto_register_fp(void)
         { &ei_fp_hsdsch_common_t3_not_implemented, { "fp.hsdsch_common_t3.not_implemented", PI_DEBUG, PI_ERROR, "HSDSCH COMMON T3 - Not implemeneted!", EXPFILL }},
         { &ei_fp_channel_type_unknown, { "fp.channel_type.unknown", PI_MALFORMED, PI_ERROR, "Unknown channel type", EXPFILL }},
         { &ei_fp_no_per_frame_info, { "fp.no_per_frame_info", PI_UNDECODED, PI_ERROR, "Can't dissect FP frame because no per-frame info was attached!", EXPFILL }},
+        { &ei_fp_no_per_conv_channel_info, { "fp.no_per_conv_channel_info", PI_UNDECODED, PI_ERROR, "Can't dissect this FP stream because no per-conversation channel info was attached!", EXPFILL }},
     };
 
     module_t *fp_module;