Fix strict aliasing of sockaddr structures

Casting from one struct sockaddr type to another breaks C’s strict
aliasing rules (variables of different types cannot alias). Fix this
cleanly by using unions of struct sockaddrs to convert between the
types (i.e. type-punning).

I wish sockaddr didn’t have to be this painful.

See:
http://gcc.gnu.org/onlinedocs/gcc-4.4.1/gcc/Optimize-Options.html#Type_002dpunning

4 files changed, 121 insertions, 72 deletions

diff --git a/agent/address.c b/agent/address.c
index e37744d..9a7fe33 100644
--- a/agent/address.c
+++ b/agent/address.c
@@ -226,20 +226,25 @@ nice_address_set_from_sockaddr (NiceAddress *addr,
 
 NICEAPI_EXPORT void
 nice_address_copy_to_sockaddr (const NiceAddress *addr,
-                               struct sockaddr *sa)
+                               struct sockaddr *_sa)
 {
-  struct sockaddr_in *sin4 = (struct sockaddr_in *)sa;
-  struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)sa;
+  union {
+    struct sockaddr *addr;
+    struct sockaddr_in *in;
+    struct sockaddr_in6 *in6;
+  } sa;
 
-  g_assert (sa);
+  sa.addr = _sa;
+
+  g_assert (_sa);
 
   switch (addr->s.addr.sa_family)
     {
     case AF_INET:
-      memcpy (sin4, &addr->s.ip4, sizeof (*sin4));
+      memcpy (sa.in, &addr->s.ip4, sizeof (*sa.in));
       break;
     case AF_INET6:
-      memcpy (sin6, &addr->s.ip6, sizeof (*sin6));
+      memcpy (sa.in6, &addr->s.ip6, sizeof (*sa.in6));
       break;
     default:
       g_return_if_reached ();
diff --git a/agent/conncheck.c b/agent/conncheck.c
index 91d2392..adda15d 100644
--- a/agent/conncheck.c
+++ b/agent/conncheck.c
@@ -2096,7 +2096,10 @@ static CandidateCheckPair *priv_process_response_check_for_peer_reflexive(NiceAg
  */
 static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream *stream, Component *component, NiceSocket *sockptr, const NiceAddress *from, NiceCandidate *local_candidate, NiceCandidate *remote_candidate, StunMessage *resp)
 {
-  struct sockaddr_storage sockaddr;
+  union {
+    struct sockaddr_storage storage;
+    struct sockaddr addr;
+  } sockaddr;
   socklen_t socklen = sizeof (sockaddr);
   GSList *i;
   StunUsageIceReturn res;
@@ -2113,7 +2116,7 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream *
 
       if (memcmp (discovery_id, response_id, sizeof(StunTransactionId)) == 0) {
         res = stun_usage_ice_conncheck_process (resp,
-            (struct sockaddr *) &sockaddr, &socklen,
+            &sockaddr.addr, &socklen,
             agent_to_ice_compatibility (agent));
         nice_debug ("Agent %p : stun_bind_process/conncheck for %p res %d "
             "(controlling=%d).", agent, p, (int)res, agent->controlling_mode);
@@ -2164,7 +2167,7 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream *
             priv_conn_check_unfreeze_related (agent, stream, p);
           } else {
             ok_pair = priv_process_response_check_for_peer_reflexive(agent,
-                stream, component, p, sockptr, (struct sockaddr *) &sockaddr,
+                stream, component, p, sockptr, &sockaddr.addr,
                 local_candidate, remote_candidate);
           }
 
@@ -2226,10 +2229,18 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, Stream *
  */
 static gboolean priv_map_reply_to_discovery_request (NiceAgent *agent, StunMessage *resp)
 {
-  struct sockaddr_storage sockaddr;
+  union {
+    struct sockaddr_storage storage;
+    struct sockaddr addr;
+  } sockaddr;
   socklen_t socklen = sizeof (sockaddr);
-  struct sockaddr_storage alternate;
+
+  union {
+    struct sockaddr_storage storage;
+    struct sockaddr addr;
+  } alternate;
   socklen_t alternatelen = sizeof (sockaddr);
+
   GSList *i;
   StunUsageBindReturn res;
   gboolean trans_found = FALSE;
@@ -2245,24 +2256,22 @@ static gboolean priv_map_reply_to_discovery_request (NiceAgent *agent, StunMessa
       stun_message_id (&d->stun_message, discovery_id);
 
       if (memcmp (discovery_id, response_id, sizeof(StunTransactionId)) == 0) {
-        res = stun_usage_bind_process (resp, (struct sockaddr *) &sockaddr,
-            &socklen, (struct sockaddr *) &alternate, &alternatelen);
+        res = stun_usage_bind_process (resp, &sockaddr.addr,
+            &socklen, &alternate.addr, &alternatelen);
         nice_debug ("Agent %p : stun_bind_process/disc for %p res %d.",
             agent, d, (int)res);
 
         if (res == STUN_USAGE_BIND_RETURN_ALTERNATE_SERVER) {
           /* handle alternate server */
           NiceAddress niceaddr;
-          nice_address_set_from_sockaddr (&niceaddr,
-              (struct sockaddr *) &alternate);
+          nice_address_set_from_sockaddr (&niceaddr, &alternate.addr);
           d->server = niceaddr;
 
           d->pending = FALSE;
         } else if (res == STUN_USAGE_BIND_RETURN_SUCCESS) {
           /* case: successful binding discovery, create a new local candidate */
           NiceAddress niceaddr;
-          nice_address_set_from_sockaddr (&niceaddr,
-              (struct sockaddr *) &sockaddr);
+          nice_address_set_from_sockaddr (&niceaddr, &sockaddr.addr);
 
           discovery_add_server_reflexive_candidate (
               d->agent,
@@ -2341,12 +2350,24 @@ priv_add_new_turn_refresh (CandidateDiscovery *cdisco, NiceCandidate *relay_cand
  */
 static gboolean priv_map_reply_to_relay_request (NiceAgent *agent, StunMessage *resp)
 {
-  struct sockaddr_storage sockaddr;
+  union {
+    struct sockaddr_storage storage;
+    struct sockaddr addr;
+  } sockaddr;
   socklen_t socklen = sizeof (sockaddr);
-  struct sockaddr_storage alternate;
+
+  union {
+    struct sockaddr_storage storage;
+    struct sockaddr addr;
+  } alternate;
   socklen_t alternatelen = sizeof (alternate);
-  struct sockaddr_storage relayaddr;
+
+  union {
+    struct sockaddr_storage storage;
+    struct sockaddr addr;
+  } relayaddr;
   socklen_t relayaddrlen = sizeof (relayaddr);
+
   uint32_t lifetime;
   uint32_t bandwidth;
   GSList *i;
@@ -2365,19 +2386,17 @@ static gboolean priv_map_reply_to_relay_request (NiceAgent *agent, StunMessage *
 
       if (memcmp (discovery_id, response_id, sizeof(StunTransactionId)) == 0) {
         res = stun_usage_turn_process (resp,
-            (struct sockaddr *) &relayaddr, &relayaddrlen,
-            (struct sockaddr *) &sockaddr, &socklen,
-            (struct sockaddr *) &alternate, &alternatelen,
+            &relayaddr.addr, &relayaddrlen,
+            &sockaddr.addr, &socklen,
+            &alternate.addr, &alternatelen,
             &bandwidth, &lifetime, agent_to_turn_compatibility (agent));
         nice_debug ("Agent %p : stun_turn_process/disc for %p res %d.",
             agent, d, (int)res);
 
         if (res == STUN_USAGE_TURN_RETURN_ALTERNATE_SERVER) {
           /* handle alternate server */
-          nice_address_set_from_sockaddr (&d->server,
-              (struct sockaddr *) &alternate);
-          nice_address_set_from_sockaddr (&d->turn->server,
-              (struct sockaddr *) &alternate);
+          nice_address_set_from_sockaddr (&d->server, &alternate.addr);
+          nice_address_set_from_sockaddr (&d->turn->server, &alternate.addr);
 
           d->pending = FALSE;
         } else if (res == STUN_USAGE_TURN_RETURN_RELAY_SUCCESS ||
@@ -2390,8 +2409,7 @@ static gboolean priv_map_reply_to_relay_request (NiceAgent *agent, StunMessage *
           if (res == STUN_USAGE_TURN_RETURN_MAPPED_SUCCESS &&
               !nice_socket_is_reliable (d->nicesock)) {
             /* We also received our mapped address */
-            nice_address_set_from_sockaddr (&niceaddr,
-                (struct sockaddr *) &sockaddr);
+            nice_address_set_from_sockaddr (&niceaddr, &sockaddr.addr);
 
             discovery_add_server_reflexive_candidate (
                 d->agent,
@@ -2401,8 +2419,7 @@ static gboolean priv_map_reply_to_relay_request (NiceAgent *agent, StunMessage *
                 d->nicesock);
           }
 
-          nice_address_set_from_sockaddr (&niceaddr,
-              (struct sockaddr *) &relayaddr);
+          nice_address_set_from_sockaddr (&niceaddr, &relayaddr.addr);
           relay_cand = discovery_add_relay_candidate (
              d->agent,
              d->stream->id,
@@ -2696,7 +2713,10 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream,
     Component *component, NiceSocket *socket, const NiceAddress *from,
     gchar *buf, guint len)
 {
-  struct sockaddr_storage sockaddr;
+  union {
+    struct sockaddr_storage storage;
+    struct sockaddr addr;
+  } sockaddr;
   uint8_t rbuf[MAX_STUN_DATAGRAM_PAYLOAD];
   ssize_t res;
   size_t rbuf_len = sizeof (rbuf);
@@ -2715,7 +2735,7 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream,
   NiceCandidate *local_candidate = NULL;
   gboolean discovery_msg = FALSE;
 
-  nice_address_copy_to_sockaddr (from, (struct sockaddr *) &sockaddr);
+  nice_address_copy_to_sockaddr (from, &sockaddr.addr);
 
   /* note: contents of 'buf' already validated, so it is
    *       a valid and fully received STUN message */
@@ -2921,7 +2941,7 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, Stream *stream,
 
     rbuf_len = sizeof (rbuf);
     res = stun_usage_ice_conncheck_create_reply (&agent->stun_agent, &req,
-        &msg, rbuf, &rbuf_len, (struct sockaddr *) &sockaddr, sizeof (sockaddr),
+        &msg, rbuf, &rbuf_len, &sockaddr.addr, sizeof (sockaddr),
         &control, agent->tie_breaker,
         agent_to_ice_compatibility (agent));
 
diff --git a/agent/interfaces.c b/agent/interfaces.c
index 2bf7d3b..a738d70 100644
--- a/agent/interfaces.c
+++ b/agent/interfaces.c
@@ -143,25 +143,30 @@ nice_interfaces_get_local_interfaces (void)
 
 
 static gboolean
-nice_interfaces_is_private_ip (const struct sockaddr *sa)
+nice_interfaces_is_private_ip (const struct sockaddr *_sa)
 {
-  if (sa->sa_family == AF_INET) {
-    struct sockaddr_in *sa4 = (struct sockaddr_in *) sa;
+  union {
+    const struct sockaddr *addr;
+    const struct sockaddr_in *in;
+  } sa;
 
+  sa.addr = _sa;
+
+  if (sa.addr->sa_family == AF_INET) {
     /* 10.x.x.x/8 */
-    if (sa4->sin_addr.s_addr >> 24 == 0x0A)
+    if (sa.in->sin_addr.s_addr >> 24 == 0x0A)
       return TRUE;
 
     /* 172.16.0.0 - 172.31.255.255 = 172.16.0.0/10 */
-    if (sa4->sin_addr.s_addr >> 20 == 0xAC1)
+    if (sa.in->sin_addr.s_addr >> 20 == 0xAC1)
       return TRUE;
 
     /* 192.168.x.x/16 */
-    if (sa4->sin_addr.s_addr >> 16 == 0xC0A8)
+    if (sa.in->sin_addr.s_addr >> 16 == 0xC0A8)
       return TRUE;
 
     /* 169.254.x.x/16  (for APIPA) */
-    if (sa4->sin_addr.s_addr >> 16 == 0xA9FE)
+    if (sa.in->sin_addr.s_addr >> 16 == 0xA9FE)
       return TRUE;
   }
   
@@ -185,6 +190,14 @@ nice_interfaces_get_local_ips (gboolean include_loopback)
   for (ifa = results; ifa; ifa = ifa->ifa_next) {
     char addr_as_string[INET6_ADDRSTRLEN+1];
 
+    union {
+      struct sockaddr *addr;
+      struct sockaddr_in *in;
+      struct sockaddr_in6 *in6;
+    } sa;
+
+    sa.addr = ifa->ifa_addr;
+
     /* no ip address from interface that is down */
     if ((ifa->ifa_flags & IFF_UP) == 0)
       continue;
@@ -192,19 +205,15 @@ nice_interfaces_get_local_ips (gboolean include_loopback)
     if (ifa->ifa_addr == NULL) {
       continue;
     } else if (ifa->ifa_addr->sa_family == AF_INET) {
-      struct sockaddr_in *sa4 = (struct sockaddr_in *) ifa->ifa_addr;
-
-      if (inet_ntop (AF_INET, &sa4->sin_addr, addr_as_string,
+      if (inet_ntop (AF_INET, &sa.in->sin_addr, addr_as_string,
               INET6_ADDRSTRLEN) == NULL)
         continue;
     } else if (ifa->ifa_addr->sa_family == AF_INET6) {
-      struct sockaddr_in6 *sa6 = (struct sockaddr_in6 *)ifa->ifa_addr;
-
       /* Skip link-local addresses, they require a scope */
-      if (IN6_IS_ADDR_LINKLOCAL (&sa6->sin6_addr))
+      if (IN6_IS_ADDR_LINKLOCAL (&sa.in6->sin6_addr))
         continue;
 
-      if (inet_ntop (AF_INET6, &sa6->sin6_addr, addr_as_string,
+      if (inet_ntop (AF_INET6, &sa.in6->sin6_addr, addr_as_string,
               INET6_ADDRSTRLEN) == NULL)
         continue;
     } else
@@ -318,7 +327,10 @@ gchar *
 nice_interfaces_get_ip_for_interface (gchar *interface_name)
 {
   struct ifreq ifr;
-  struct sockaddr_in *sa;
+  union {
+    struct sockaddr *addr;
+    struct sockaddr_in *in;
+  } sa;
   gint sockfd;
 
 
@@ -339,9 +351,9 @@ nice_interfaces_get_ip_for_interface (gchar *interface_name)
   }
 
   close (sockfd);
-  sa = (struct sockaddr_in *) &ifr.ifr_addr;
-  nice_debug ("Address for %s: %s", interface_name, inet_ntoa (sa->sin_addr));
-  return g_strdup (inet_ntoa (sa->sin_addr));
+  sa.addr = &ifr.ifr_addr;
+  nice_debug ("Address for %s: %s", interface_name, inet_ntoa (sa.in->sin_addr));
+  return g_strdup (inet_ntoa (sa.in->sin_addr));
 }
 
 #else /* G_OS_UNIX */
diff --git a/agent/pseudotcp.c b/agent/pseudotcp.c
index 632eb28..fe8dcc1 100644
--- a/agent/pseudotcp.c
+++ b/agent/pseudotcp.c
@@ -794,32 +794,36 @@ packet(PseudoTcpSocket *self, guint32 seq, guint8 flags,
 {
   PseudoTcpSocketPrivate *priv = self->priv;
   guint32 now = get_current_time();
-  guint8 buffer[MAX_PACKET];
+  union {
+    guint8 u8[MAX_PACKET];
+    guint16 u16[MAX_PACKET / 2];
+    guint32 u32[MAX_PACKET / 4];
+  } buffer;
   PseudoTcpWriteResult wres = WR_SUCCESS;
 
   g_assert(HEADER_SIZE + len <= MAX_PACKET);
 
-  *((guint32 *) buffer) = htonl(priv->conv);
-  *((guint32 *) (buffer + 4)) = htonl(seq);
-  *((guint32 *) (buffer + 8)) = htonl(priv->rcv_nxt);
-  buffer[12] = 0;
-  buffer[13] = flags;
-  *((guint16 *) (buffer + 14)) = htons((guint16)priv->rcv_wnd);
+  *buffer.u32 = htonl(priv->conv);
+  *(buffer.u32 + 1) = htonl(seq);
+  *(buffer.u32 + 2) = htonl(priv->rcv_nxt);
+  buffer.u8[12] = 0;
+  buffer.u8[13] = flags;
+  *(buffer.u16 + 7) = htons((guint16)priv->rcv_wnd);
 
   // Timestamp computations
-  *((guint32 *) (buffer + 16)) = htonl(now);
-  *((guint32 *) (buffer + 20)) = htonl(priv->ts_recent);
+  *(buffer.u32 + 4) = htonl(now);
+  *(buffer.u32 + 5) = htonl(priv->ts_recent);
   priv->ts_lastack = priv->rcv_nxt;
 
   if (data != NULL)
-    memcpy(buffer + HEADER_SIZE, data, len);
+    memcpy(buffer.u8 + HEADER_SIZE, data, len);
 
   DEBUG (PSEUDO_TCP_DEBUG_VERBOSE, "<-- <CONV=%d><FLG=%d><SEQ=%d:%d><ACK=%d>"
       "<WND=%d><TS=%d><TSR=%d><LEN=%d>",
       priv->conv, (unsigned)flags, seq, seq + len, priv->rcv_nxt, priv->rcv_wnd,
       now % 10000, priv->ts_recent % 10000, len);
 
-  wres = priv->callbacks.WritePacket(self, (gchar *) buffer, len + HEADER_SIZE,
+  wres = priv->callbacks.WritePacket(self, (gchar *) buffer.u8, len + HEADER_SIZE,
                                      priv->callbacks.user_data);
   /* Note: When data is NULL, this is an ACK packet.  We don't read the
      return value for those, and thus we won't retry.  So go ahead and treat
@@ -839,23 +843,31 @@ packet(PseudoTcpSocket *self, guint32 seq, guint8 flags,
 }
 
 static gboolean
-parse(PseudoTcpSocket *self, const guint8 * buffer, guint32 size)
+parse(PseudoTcpSocket *self, const guint8 * _buffer, guint32 size)
 {
   Segment seg;
 
+  union {
+    const guint8 *u8;
+    const guint16 *u16;
+    const guint32 *u32;
+  } buffer;
+
+  buffer.u8 = _buffer;
+
   if (size < 12)
     return FALSE;
 
-  seg.conv = ntohl(*(guint32 *)buffer);
-  seg.seq = ntohl(*(guint32 *)(buffer + 4));
-  seg.ack = ntohl(*(guint32 *)(buffer + 8));
-  seg.flags = buffer[13];
-  seg.wnd = ntohs(*(guint16 *)(buffer + 14));
+  seg.conv = ntohl(*buffer.u32);
+  seg.seq = ntohl(*(buffer.u32 + 1));
+  seg.ack = ntohl(*(buffer.u32 + 2));
+  seg.flags = buffer.u8[13];
+  seg.wnd = ntohs(*(buffer.u16 + 7));
 
-  seg.tsval = ntohl(*(guint32 *)(buffer + 16));
-  seg.tsecr = ntohl(*(guint32 *)(buffer + 20));
+  seg.tsval = ntohl(*(buffer.u32 + 4));
+  seg.tsecr = ntohl(*(buffer.u32 + 5));
 
-  seg.data = ((gchar *)buffer) + HEADER_SIZE;
+  seg.data = ((gchar *)buffer.u8) + HEADER_SIZE;
   seg.len = size - HEADER_SIZE;
 
   DEBUG (PSEUDO_TCP_DEBUG_VERBOSE, "--> <CONV=%d><FLG=%d><SEQ=%d:%d><ACK=%d>"