WCCP: Fix up storing and decoding of IP addresses.

Try to decode as many IP addresses as we can, even if they don't fit in the
table we store them in.

Only add IP addresses in the table once.  We could theoretically reallocate
the table but the original code seems to assume the addresses should (in
non-fuzzed captures) only show up once per PDU.  This part of the change fixes
the fuzz failure.

Bug: 11153
Change-Id: I56b9854ac1342080c9f32699a3f97750fa335696
Reviewed-on: https://code.wireshark.org/review/8748
Petri-Dish: Jeff Morriss <jeff.morriss.ws@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

1 files changed, 60 insertions, 69 deletions

diff --git a/epan/dissectors/packet-wccp.c b/epan/dissectors/packet-wccp.c
index b2c99a532f..27008a19c6 100644
--- a/epan/dissectors/packet-wccp.c
+++ b/epan/dissectors/packet-wccp.c
@@ -1836,110 +1836,101 @@ dissect_wccp2_command_extension(tvbuff_t *tvb, int offset,
    once to get the tables, once to display them
 */
 static gint
-dissect_wccp2r1_address_table_info(tvbuff_t *tvb, int offset,
-                                   int length, packet_info *pinfo, proto_tree *info_tree)
+dissect_wccp2r1_address_table_info(tvbuff_t *tvb, int offset, int length,
+                                   packet_info *pinfo, proto_tree *info_tree)
 {
   guint16 address_length;
   guint32 i;
-  proto_tree *element_tree=NULL;
-  proto_item *tf=NULL;
-
-  wccp_wccp_address_table.in_use = TRUE;
+  gint16 family;
+  guint16 table_length;
+  proto_tree *element_tree;
+  proto_item *tf;
 
   if (length < 2*4)
     return length - 2*4;
 
-
-  wccp_wccp_address_table.family = tvb_get_ntohs(tvb,offset);
-
-  proto_tree_add_item(info_tree, hf_address_table_family, tvb,
-                        offset, 2, ENC_BIG_ENDIAN);
+  family = tvb_get_ntohs(tvb, offset);
+  proto_tree_add_item(info_tree, hf_address_table_family, tvb, offset, 2, ENC_BIG_ENDIAN);
   EAT_AND_CHECK(2,2);
 
-  address_length = tvb_get_ntohs(tvb,offset);
+  address_length = tvb_get_ntohs(tvb, offset);
   proto_tree_add_item(info_tree, hf_address_table_address_length, tvb, offset, 2, ENC_BIG_ENDIAN);
   EAT_AND_CHECK(2,2);
 
-  wccp_wccp_address_table.table_length =  tvb_get_ntohl(tvb,offset);
+  table_length =  tvb_get_ntohl(tvb, offset);
   tf = proto_tree_add_item(info_tree, hf_address_table_length, tvb, offset, 4, ENC_BIG_ENDIAN);
   element_tree = proto_item_add_subtree(tf, ett_table_element);
-
   EAT(4);
 
-  /* check if the length is valid and allocate the tables if needed*/
-  switch (wccp_wccp_address_table.family) {
-  case 1:
-    if (wccp_wccp_address_table.table_ipv4 == NULL)
-      wccp_wccp_address_table.table_ipv4 = (guint32 *)
-        wmem_alloc0(pinfo->pool, wccp_wccp_address_table.table_length * 4);
-    if (address_length != 4) {
-      expert_add_info_format(pinfo, tf, &ei_wccp_length_bad,
-                             "The Address length must be 4, but I found  %d for IPv4 addresses. Correcting this.",
-                             address_length);
-      address_length = 4;
-    }
-    break;
-  case 2:
-    if (wccp_wccp_address_table.table_ipv6 == NULL)
-      wccp_wccp_address_table.table_ipv6 = (struct e_in6_addr *)
-        wmem_alloc0(pinfo->pool, wccp_wccp_address_table.table_length * sizeof(struct e_in6_addr));
-    if (address_length != 16) {
-      expert_add_info_format(pinfo, tf, &ei_wccp_length_bad,
-                             "The Address length must be 16, but I found %d for IPv6 addresses.  Correcting this",
-                             address_length);
-      address_length=16;
-    }
-    break;
-  default:
-    expert_add_info_format(pinfo, tf, &ei_wccp_address_table_family_unknown,
-                    "Unknown address family: %d", wccp_wccp_address_table.family);
-  };
-
-  /* now read the addresses and print/store them */
-
-  for(i=0; i<wccp_wccp_address_table.table_length; i++) {
-    const gchar* addr;
+  if (wccp_wccp_address_table.in_use == FALSE) {
+    wccp_wccp_address_table.in_use = TRUE;
+    wccp_wccp_address_table.family = family;
+    wccp_wccp_address_table.table_length =  table_length;
 
-    /* do we have space? */
-    if (length < address_length) {
-      expert_add_info_format(pinfo, tf, &ei_wccp_length_bad, "Ran out of space to decode");
+    /* check if the length is valid and allocate the tables if needed */
+    switch (wccp_wccp_address_table.family) {
+    case 1:
+      if (wccp_wccp_address_table.table_ipv4 == NULL)
+        wccp_wccp_address_table.table_ipv4 = (guint32 *)
+          wmem_alloc0(pinfo->pool, wccp_wccp_address_table.table_length * 4);
+      if (address_length != 4) {
+        expert_add_info_format(pinfo, tf, &ei_wccp_length_bad,
+                               "The Address length must be 4, but I found %d for IPv4 addresses. Correcting this.",
+                               address_length);
+        address_length = 4;
+      }
+      break;
+    case 2:
+      if (wccp_wccp_address_table.table_ipv6 == NULL)
+        wccp_wccp_address_table.table_ipv6 = (struct e_in6_addr *)
+          wmem_alloc0(pinfo->pool, wccp_wccp_address_table.table_length * sizeof(struct e_in6_addr));
+      if (address_length != 16) {
+        expert_add_info_format(pinfo, tf, &ei_wccp_length_bad,
+                               "The Address length must be 16, but I found %d for IPv6 addresses. Correcting this.",
+                               address_length);
+        address_length = 16;
+      }
+      break;
+    default:
+      expert_add_info_format(pinfo, tf, &ei_wccp_address_table_family_unknown,
+                      "Unknown address family: %d", wccp_wccp_address_table.family);
+    };
+  }
 
-      /* first clean up: */
-      wccp_wccp_address_table.in_use = FALSE;
-      wccp_wccp_address_table.family = -1;
-      wccp_wccp_address_table.version = -1;
-      wccp_wccp_address_table.table_length = 0;
-      wccp_wccp_address_table.table_ipv4 = (guint32 *) NULL;
-      wccp_wccp_address_table.table_ipv6 = (struct e_in6_addr *)NULL;
+  /* now read the addresses and print/store them */
 
-      /* ABORT The decode */
-      return length - address_length;
-    }
+  for(i=0; i<table_length; i++) {
+    const gchar *addr;
 
-    switch (wccp_wccp_address_table.family) {
+    switch (family) {
     case 1:
       /* IPv4 */
       addr  =  tvb_ip_to_str(tvb, offset);
-      if (wccp_wccp_address_table.table_ipv4 != NULL)
-        wccp_wccp_address_table.table_ipv4[i] = tvb_get_ntohl(tvb,offset);
+      if ((wccp_wccp_address_table.table_ipv4 != NULL) && ((address_length * i) < wccp_wccp_address_table.table_length))
+        wccp_wccp_address_table.table_ipv4[i] = tvb_get_ntohl(tvb, offset);
       break;
     case 2:
       /* IPv6 */
-      addr = tvb_ip6_to_str(tvb,offset);
-      if (wccp_wccp_address_table.table_ipv6 != NULL)
+      addr = tvb_ip6_to_str(tvb, offset);
+      if ((wccp_wccp_address_table.table_ipv6 != NULL) && ((address_length * i) < wccp_wccp_address_table.table_length))
         tvb_get_ipv6(tvb, offset, &(wccp_wccp_address_table.table_ipv6[i]));
       break;
     default:
-      addr = wmem_strdup_printf(wmem_packet_scope(), "unknown family");
+      addr = wmem_strdup_printf(wmem_packet_scope(), "unknown family %d", wccp_wccp_address_table.family);
     };
 
     if (element_tree) {
-      proto_tree_add_string_format_value(element_tree, hf_address_table_element, tvb,
-                                         offset, address_length, addr,
-                                         "%d: %s", i+1, addr);
+      proto_item *pi;
+
+      pi = proto_tree_add_string_format_value(element_tree, hf_address_table_element, tvb,
+                                              offset, address_length, addr,
+                                              "%d: %s", i+1, addr);
+      if ((address_length * i) > wccp_wccp_address_table.table_length)
+        expert_add_info_format(pinfo, pi, &ei_wccp_length_bad, "Ran out of space to store address");
     }
     EAT(address_length);
   }
+
   return length;
 }