[BUG] Fix misaligned sockaddr_in access in SocketAddr

SocketAddr(u_long, uint16_t) bound a sockaddr_in reference to the sockaddr
storage through a reinterpret_cast. sockaddr is not aligned for a sockaddr_in
on every ABI (alignment 1 on macOS and 2 on Linux versus 4), so the access is
undefined and UBSan reports it:

  socket_tools.h:180:29: runtime error: reference binding to misaligned
  address 0x00016bda6551 for type 'sockaddr_in', which requires 4 byte
  alignment

Store the address in a sockaddr_storage, which is the socket API's type for an
address of any family and is aligned for all of them, and build the IPv4
address in a local sockaddr_in that is copied into the storage, as the string
constructor already does. Replace the sockaddr conversion operators with
addr() and addr_len(): the storage is larger than a sockaddr_in and passing
its full size as the address length to bind() or connect() is rejected on
some platforms, so addr_len() is the length of a sockaddr_in, the only family
stored. getsockname() and accept() keep passing the full storage size, which
is the buffer capacity for those output parameters.

The default constructor sets AF_UNSPEC explicitly, and the tests read the
family through ss_family. New tests cover the integer constructor at an
address only sockaddr_storage alignment can guarantee, the accessors, and a
bind/getsockname/connect/accept round trip on the loopback interface.

Fixes #4307

Author: JonSnow1807

CHANGELOG.md

@@ -15,6 +15,10 @@ Increment the:
 
 ## [Unreleased]
 
+* [BUG] Store `SocketAddr` in a `sockaddr_storage` and pass `addr()` and
+  `addr_len()` to the socket API, fixing a misaligned `sockaddr_in` access
+  [#4307](https://github.com/open-telemetry/opentelemetry-cpp/issues/4307)
+
 ## [1.30.0] 2026-10-22
 
 This release contains:

ext/include/opentelemetry/ext/http/server/socket_tools.h

@@ -167,20 +167,25 @@ struct SocketAddr
 {
   static u_long const Loopback = 0x7F000001;
 
-  sockaddr m_data{};
+  // sockaddr_storage is the socket API's type for holding an address of any family: it is large
+  // enough and aligned strictly enough for a sockaddr_in, which a plain sockaddr is not on every
+  // ABI. Addresses are copied in and out of it with memcpy; see addr() and addr_len() for how it
+  // is passed to the socket API.
+  sockaddr_storage m_data{};
 
   /// <summary>
   /// SocketAddr constructor
   /// </summary>
   /// <returns>SocketAddr</returns>
-  SocketAddr() {}
+  SocketAddr() { m_data.ss_family = AF_UNSPEC; }
 
   SocketAddr(u_long addr, uint16_t port)
   {
-    sockaddr_in &inet4    = reinterpret_cast<sockaddr_in &>(m_data);
+    sockaddr_in inet4{};
     inet4.sin_family      = AF_INET;
     inet4.sin_port        = htons(port);
     inet4.sin_addr.s_addr = htonl(addr);
+    std::memcpy(&m_data, &inet4, sizeof(inet4));
   }
 
   /// Parses an IPv4 address in "host" or "host:port" form. Host parsing follows the platform's
@@ -264,13 +269,18 @@ struct SocketAddr
     }
   }
 
-  operator sockaddr *() { return &m_data; }
+  /// The stored address as the socket API takes it. Only AF_INET addresses are stored, so
+  /// addr_len() is the length of a sockaddr_in rather than of the whole storage: bind() and
+  /// connect() reject a length that is too large for the address family on some platforms.
+  sockaddr *addr() { return reinterpret_cast<sockaddr *>(&m_data); }
 
-  operator const sockaddr *() const { return &m_data; }
+  const sockaddr *addr() const { return reinterpret_cast<const sockaddr *>(&m_data); }
+
+  socklen_t addr_len() const { return static_cast<socklen_t>(sizeof(sockaddr_in)); }
 
   int port() const
   {
-    switch (m_data.sa_family)
+    switch (m_data.ss_family)
     {
       case AF_INET: {
         // Copy out rather than binding a sockaddr_in glvalue to sockaddr storage, which is an
@@ -289,7 +299,7 @@ struct SocketAddr
   {
     std::ostringstream os;
 
-    switch (m_data.sa_family)
+    switch (m_data.ss_family)
     {
       case AF_INET: {
         sockaddr_in inet4{};
@@ -302,23 +312,22 @@ struct SocketAddr
       }
 
       default:
-        os << "[?AF?" << m_data.sa_family << ']';
+        os << "[?AF?" << m_data.ss_family << ']';
     }
     return os.str();
   }
 };
 
-// The parser memcpys a sockaddr_in into m_data, and the socket syscalls pass sizeof(SocketAddr)
-// as the address length. This wrapper is IPv4-only, so require sockaddr and sockaddr_in to have
-// the exact same size rather than trusting every ABI: passing an address length that is too large
-// for the family is a documented EINVAL for connect()/bind(). Exact equality also keeps the memcpy
-// safe. Together with the assertion below, sizeof(SocketAddr) == sizeof(sockaddr_in).
-static_assert(sizeof(sockaddr) == sizeof(sockaddr_in),
-              "SocketAddr is IPv4-only: sockaddr and sockaddr_in must have identical size");
-static_assert(offsetof(sockaddr, sa_family) == offsetof(sockaddr_in, sin_family),
-              "sockaddr and sockaddr_in must place the address family at the same offset");
-static_assert(sizeof(SocketAddr) == sizeof(sockaddr),
-              "SocketAddr must add no storage beyond its sockaddr, since syscalls use its size");
+// The constructors memcpy a sockaddr_in into m_data, port() and toString() memcpy it back out, and
+// the address family is read through ss_family. Those are only well defined if the storage is
+// large enough and aligned strictly enough for a sockaddr_in and places the address family where
+// a sockaddr_in does.
+static_assert(sizeof(sockaddr_storage) >= sizeof(sockaddr_in),
+              "sockaddr_storage must be large enough to hold a sockaddr_in");
+static_assert(alignof(sockaddr_storage) >= alignof(sockaddr_in),
+              "sockaddr_storage must be aligned strictly enough for a sockaddr_in");
+static_assert(offsetof(sockaddr_storage, ss_family) == offsetof(sockaddr_in, sin_family),
+              "sockaddr_storage and sockaddr_in must place the address family at the same offset");
 
 /// <summary>
 /// Encapsulation of a socket (non-exclusive ownership)
@@ -388,7 +397,7 @@ struct Socket
   bool connect(SocketAddr const &addr)
   {
     assert(m_sock != Invalid);
-    return (::connect(m_sock, addr, sizeof(addr)) == 0);
+    return (::connect(m_sock, addr.addr(), addr.addr_len()) == 0);
   }
 
   void close()
@@ -418,18 +427,18 @@ struct Socket
   bool bind(SocketAddr const &addr)
   {
     assert(m_sock != Invalid);
-    return (::bind(m_sock, addr, sizeof(addr)) == 0);
+    return (::bind(m_sock, addr.addr(), addr.addr_len()) == 0);
   }
 
   bool getsockname(SocketAddr &addr) const
   {
     assert(m_sock != Invalid);
 #ifdef _WIN32
-    int addrlen = sizeof(addr);
+    int addrlen = sizeof(addr.m_data);
 #else
-    socklen_t addrlen = sizeof(addr);
+    socklen_t addrlen = sizeof(addr.m_data);
 #endif
-    return (::getsockname(m_sock, addr, &addrlen) == 0);
+    return (::getsockname(m_sock, addr.addr(), &addrlen) == 0);
   }
 
   bool listen(int backlog)
@@ -442,11 +451,11 @@ struct Socket
   {
     assert(m_sock != Invalid);
 #ifdef _WIN32
-    int addrlen = sizeof(caddr);
+    int addrlen = sizeof(caddr.m_data);
 #else
-    socklen_t addrlen = sizeof(caddr);
+    socklen_t addrlen = sizeof(caddr.m_data);
 #endif
-    csock = ::accept(m_sock, caddr, &addrlen);
+    csock = ::accept(m_sock, caddr.addr(), &addrlen);
     return !csock.invalid();
   }
 

ext/test/http/socket_tools_test.cc

@@ -2,9 +2,12 @@
 // SPDX-License-Identifier: Apache-2.0
 
 #include <gtest/gtest.h>
+#include <stddef.h>
+#include <cstdint>
 #include <string>
 #ifndef _WIN32
-#  include <sys/socket.h>  // for sockaddr, AF_UNSPEC, AF_INET
+#  include <netinet/in.h>  // for sockaddr_in, IPPROTO_TCP
+#  include <sys/socket.h>  // for sockaddr, AF_UNSPEC, AF_INET, SOCK_STREAM
 #endif
 
 #include "opentelemetry/ext/http/server/socket_tools.h"
@@ -17,14 +20,75 @@ namespace
 // corrupted while port() happens to still return -1.
 void ExpectInvalid(const SocketTools::SocketAddr &addr)
 {
-  EXPECT_EQ(addr.m_data.sa_family, AF_UNSPEC);
+  EXPECT_EQ(addr.m_data.ss_family, AF_UNSPEC);
   EXPECT_EQ(addr.port(), -1);
 }
 
+TEST(SocketAddrTest, DefaultsToUnspecified)
+{
+  SocketTools::SocketAddr addr;
+  EXPECT_EQ(addr.m_data.ss_family, AF_UNSPEC);
+  EXPECT_EQ(addr.port(), -1);
+}
+
+// The storage must be aligned for every address type copied into it. Placing the address after a
+// single byte lets the compiler put it wherever its own alignment allows; with a sockaddr
+// (alignment 1 or 2) as the storage that was an address a sockaddr_in (alignment 4) cannot live
+// at, which UBSan's alignment check reported for this constructor.
+TEST(SocketAddrTest, ConstructsFromHostOrderAddressAndPort)
+{
+  struct Padded
+  {
+    char pad = 0;
+    SocketTools::SocketAddr addr{SocketTools::SocketAddr::Loopback, 8800};
+  } padded;
+  EXPECT_EQ(reinterpret_cast<std::uintptr_t>(&padded.addr.m_data) % alignof(sockaddr_in), 0u);
+  EXPECT_EQ(padded.addr.m_data.ss_family, AF_INET);
+  EXPECT_EQ(padded.addr.port(), 8800);
+  EXPECT_EQ(padded.addr.toString(), "127.0.0.1:8800");
+}
+
+// bind() and connect() take addr() and addr_len(). The storage is larger than an IPv4 address, and
+// passing its full size as the address length is rejected with EINVAL on some platforms.
+TEST(SocketAddrTest, ExposesAddressAndLengthForTheSocketApi)
+{
+  SocketTools::SocketAddr addr(SocketTools::SocketAddr::Loopback, 8800);
+  EXPECT_EQ(static_cast<const void *>(addr.addr()), static_cast<const void *>(&addr.m_data));
+  EXPECT_EQ(static_cast<size_t>(addr.addr_len()), sizeof(sockaddr_in));
+}
+
+TEST(SocketTest, BindsConnectsAndAcceptsOnLoopback)
+{
+  SocketTools::Socket listener(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+  ASSERT_FALSE(listener.invalid());
+  ASSERT_TRUE(listener.bind(SocketTools::SocketAddr(SocketTools::SocketAddr::Loopback, 0)));
+  ASSERT_TRUE(listener.listen(1));
+
+  SocketTools::SocketAddr bound;
+  ASSERT_TRUE(listener.getsockname(bound));
+  EXPECT_EQ(bound.m_data.ss_family, AF_INET);
+  ASSERT_GT(bound.port(), 0);
+
+  SocketTools::Socket client(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+  ASSERT_FALSE(client.invalid());
+  ASSERT_TRUE(client.connect(SocketTools::SocketAddr(SocketTools::SocketAddr::Loopback,
+                                                     static_cast<uint16_t>(bound.port()))));
+
+  SocketTools::Socket accepted;
+  SocketTools::SocketAddr peer;
+  ASSERT_TRUE(listener.accept(accepted, peer));
+  EXPECT_EQ(peer.m_data.ss_family, AF_INET);
+  EXPECT_EQ(peer.toString().substr(0, 10), "127.0.0.1:");
+
+  accepted.close();
+  client.close();
+  listener.close();
+}
+
 TEST(SocketAddrTest, ParsesHostAndPort)
 {
   SocketTools::SocketAddr addr("127.0.0.1:8800");
-  EXPECT_EQ(addr.m_data.sa_family, AF_INET);
+  EXPECT_EQ(addr.m_data.ss_family, AF_INET);
   EXPECT_EQ(addr.port(), 8800);
   EXPECT_EQ(addr.toString(), "127.0.0.1:8800");
 }
@@ -57,7 +121,7 @@ TEST(SocketAddrTest, ParsesHostWithoutPort)
 TEST(SocketAddrTest, ParsesLegitimateZeroPort)
 {
   SocketTools::SocketAddr addr("127.0.0.1:0");
-  EXPECT_EQ(addr.m_data.sa_family, AF_INET);
+  EXPECT_EQ(addr.m_data.ss_family, AF_INET);
   EXPECT_EQ(addr.port(), 0);
   EXPECT_EQ(addr.toString(), "127.0.0.1:0");
 }
@@ -67,7 +131,7 @@ TEST(SocketAddrTest, ParsesLegitimateZeroPort)
 TEST(SocketAddrTest, AcceptsLeadingZeroPort)
 {
   SocketTools::SocketAddr addr("127.0.0.1:080");
-  EXPECT_EQ(addr.m_data.sa_family, AF_INET);
+  EXPECT_EQ(addr.m_data.ss_family, AF_INET);
   EXPECT_EQ(addr.port(), 80);
 }