Add checks to address setting routines.

Fail if:

	1) you have an AT_NONE address with data;

	2) you have a non-AT_NONE address with a zero length and a
	   non-null data pointer, or with a non-zero length and a null
	   data pointer.

When comparing addresses for equality, just make sure the types are the
same, the lengths are the same and, if the lengths are non-zero, the
data is the same; don't treat AT_NONE specially - the "lengths are
non-zero" check will make sure we do the right thing.

Make sure when we create an AT_NONE address it has a zero length and
null data pointer.

Change-Id: I5c452ef0d140c2d9aef3004f1cfd124a95b78fb2
Reviewed-on: https://code.wireshark.org/review/15839
Reviewed-by: Guy Harris <guy@alum.mit.edu>

3 files changed, 39 insertions, 23 deletions

diff --git a/epan/address.h b/epan/address.h
index 5a6ed9bad0..31eef6a158 100644
--- a/epan/address.h
+++ b/epan/address.h
@@ -91,6 +91,15 @@ clear_address(address *addr)
  */
 static inline void
 set_address(address *addr, int addr_type, int addr_len, const void *addr_data) {
+    if (addr_len == 0) {
+        /* Zero length must mean no data */
+        g_assert(addr_data == NULL);
+    } else {
+        /* Must not be AT_NONE - AT_NONE must have no data */
+        g_assert(addr_type != AT_NONE);
+        /* Make sure we *do* have data */
+        g_assert(addr_data != NULL);
+    }
     addr->type = addr_type;
     addr->len  = addr_len;
     addr->data = addr_data;
@@ -116,9 +125,11 @@ static inline void
 set_address_tvb(address *addr, int addr_type, int addr_len, tvbuff_t *tvb, int offset) {
     const void *p;
 
-    if (addr_len != 0)
+    if (addr_len != 0) {
+        /* Must not be AT_NONE - AT_NONE must have no data */
+        g_assert(addr_type != AT_NONE);
         p = tvb_get_ptr(tvb, offset, addr_len);
-    else
+    } else
         p = NULL;
     set_address(addr, addr_type, addr_len, p);
 }
@@ -139,11 +150,16 @@ alloc_address_wmem(wmem_allocator_t *scope, address *addr,
     g_assert(addr);
     clear_address(addr);
     addr->type = addr_type;
-    if (addr_type == AT_NONE || addr_len <= 0 || addr_data == NULL) {
-        g_assert(addr_len <= 0);
+    if (addr_len == 0) {
+        /* Zero length must mean no data */
         g_assert(addr_data == NULL);
+        /* Nothing to copy */
         return;
     }
+    /* Must not be AT_NONE - AT_NONE must have no data */
+    g_assert(addr_type != AT_NONE);
+    /* Make sure we *do* have data to copy */
+    g_assert(addr_data != NULL);
     addr->data = addr->priv = wmem_memdup(scope, addr_data, addr_len);
     addr->len = addr_len;
 }
@@ -189,9 +205,9 @@ cmp_address(const address *addr1, const address *addr2) {
 /** Check two addresses for equality.
  *
  * Given two addresses, return "true" if they're equal, "false" otherwise.
- * Addresses are equal only if they have the same type; if the type is
- * AT_NONE, they are then equal, otherwise they must have the same
- * amount of data and the data must be the same.
+ * Addresses are equal only if they have the same type and length; if the
+ * length is zero, they are then equal, otherwise the data must be the
+ * same.
  *
  * @param addr1 [in] The first address to compare.
  * @param addr2 [in] The second address to compare.
@@ -199,13 +215,16 @@ cmp_address(const address *addr1, const address *addr2) {
  */
 static inline gboolean
 addresses_equal(const address *addr1, const address *addr2) {
-    if (addr1->type == addr2->type
-            && ( addr1->type == AT_NONE
-                 || ( addr1->len == addr2->len
-                      && memcmp(addr1->data, addr2->data, addr1->len) == 0
-                      )
-                 )
-            ) return TRUE;
+    /*
+     * memcmp(NULL, NULL, 0) is *not* guaranteed to work, so
+     * if both addresses are zero-length, don't compare them
+     * (there's nothing to compare, so they're equal).
+     */
+    if (addr1->type == addr2->type &&
+        addr1->len == addr2->len &&
+        (addr1->len == 0 ||
+         memcmp(addr1->data, addr2->data, addr1->len) == 0))
+        return TRUE;
     return FALSE;
 }
 
diff --git a/epan/dissectors/packet-iax2.c b/epan/dissectors/packet-iax2.c
index 6687fabf76..47d77c545d 100644
--- a/epan/dissectors/packet-iax2.c
+++ b/epan/dissectors/packet-iax2.c
@@ -1517,14 +1517,13 @@ static guint32 dissect_iax2_command(tvbuff_t *tvb, guint32 offset,
                                     iax_packet_data *iax_packet)
 {
   guint8         csub = tvb_get_guint8(tvb, offset);
-  guint8         address_data[MAX_ADDRESS];
   proto_item*    ti;
   iax2_ie_data   ie_data;
   iax_call_data *iax_call;
 
   ie_data.peer_address.type = AT_NONE;
   ie_data.peer_address.len  = 0;
-  ie_data.peer_address.data = address_data;
+  ie_data.peer_address.data = NULL;
   ie_data.peer_ptype        = PT_NONE;
   ie_data.peer_port         = 0;
   ie_data.peer_callno       = 0;
diff --git a/epan/dissectors/packet-sflow.c b/epan/dissectors/packet-sflow.c
index f9a0047200..991f283e8c 100644
--- a/epan/dissectors/packet-sflow.c
+++ b/epan/dissectors/packet-sflow.c
@@ -765,7 +765,7 @@ dissect_sflow_245_address_type(tvbuff_t *tvb, packet_info *pinfo,
         break;
     default:
         /* Invalid address type, or a type we don't understand; we don't
-           know the length. W e treat it as having no contents; that
+           know the length. We treat it as having no contents; that
            doesn't trap us in an endless loop, as we at least include
            the address type and thus at least advance the offset by 4.
            Note that we have a problem, though. */
@@ -776,6 +776,9 @@ dissect_sflow_245_address_type(tvbuff_t *tvb, packet_info *pinfo,
 
     if (addr) {
         switch (len) {
+        default:
+            clear_address(addr);
+            break;
         case 4:
             set_address_tvb(addr, AT_IPv4, len, tvb, offset);
             break;
@@ -2346,16 +2349,12 @@ dissect_sflow_245(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *dat
        /* Unknown version; assume it's not an sFlow packet. */
        return 0;
     }
+
     sflow_addr_type = tvb_get_ntohl(tvb, offset + 4);
     switch (sflow_addr_type) {
         case ADDR_TYPE_UNKNOWN:
-            addr_details.type = AT_NONE;
-            break;
         case ADDR_TYPE_IPV4:
-            addr_details.type = AT_IPv4;
-            break;
         case ADDR_TYPE_IPV6:
-            addr_details.type = AT_IPv6;
             break;
 
         default:
@@ -2365,7 +2364,6 @@ dissect_sflow_245(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *dat
              */
             return 0;
     }
-
     /* Make entries in Protocol column and Info column on summary display */
     col_set_str(pinfo->cinfo, COL_PROTOCOL, "sFlow");