Skip to content

fix(ext/http): avoid misaligned sockaddr_in access in SocketAddr constructor #4629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Shubhammehta2008 wants to merge 3 commits into
base: main
Choose a base branch
from
+20 −30
Open

fix(ext/http): avoid misaligned sockaddr_in access in SocketAddr constructor #4629

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f208c6
fix(ext/http): avoid misaligned sockaddr_in access in SocketAddr cons…
Shubhammehta2008 Sep 22, 2026
f5463a6
fix(ext/http): use sockaddr_storage for SocketAddr storage
Shubhammehta2008 Sep 23, 2026
18513cd
Merge branch 'main' into fix-socketaddr-misaligned-cast
Shubhammehta2008 Sep 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 20 additions & 30 deletions ext/include/opentelemetry/ext/http/server/socket_tools.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ struct SocketAddr
{
static u_long const Loopback = 0x7F000001;

sockaddr m_data{};
sockaddr_storage m_data{};

/// <summary>
/// SocketAddr constructor
Expand All @@ -189,19 +189,13 @@ struct SocketAddr
/// input leaves the address at AF_UNSPEC, for which port() returns -1.
SocketAddr(char const *addr)
{
// One parser for every platform: inet_pton (Winsock provides it since Vista) plus a strict
// decimal port. This avoids WSAStringToAddress, whose grammar and default-component filling
// differ from the POSIX path. Parse into a local sockaddr_in and commit with memcpy only on
// success, which keeps m_data at AF_UNSPEC on failure and avoids accessing the sockaddr
// storage through a sockaddr_in glvalue (an alignment/type-access issue tracked in #4307).
if (addr == nullptr)
{
LOG_WARN("SocketAddr: cannot parse a null address");
return; // m_data is already AF_UNSPEC, so port() reports -1.
}

sockaddr_in parsed{};
parsed.sin_family = AF_INET;
sockaddr_in &parsed = reinterpret_cast<sockaddr_in &>(m_data);

char const *colon = std::strchr(addr, ':');
char const *hostEnd = colon ? colon : addr + std::strlen(addr);
Expand Down Expand Up @@ -253,30 +247,27 @@ struct SocketAddr

if (ok)
{
std::memcpy(&m_data, &parsed, sizeof(parsed));
parsed.sin_family = AF_INET;
}
else
{
// Leave m_data at AF_UNSPEC; port() returns -1 so callers can tell a parse failure from a
// real endpoint, including the legitimate ":0". Do not echo the raw input, which may be
// arbitrarily long.
// Reset on failure
std::memset(&m_data, 0, sizeof(m_data));
LOG_WARN("SocketAddr: cannot parse address");
}
}

operator sockaddr *() { return &m_data; }
operator sockaddr *() { return reinterpret_cast<sockaddr *>(&m_data); }

operator const sockaddr *() const { return &m_data; }
operator const sockaddr *() const { return reinterpret_cast<const sockaddr *>(&m_data); }

int port() const
{
switch (m_data.sa_family)
const sockaddr &sa = reinterpret_cast<const sockaddr &>(m_data);
switch (sa.sa_family)
{
case AF_INET: {
// Copy out rather than binding a sockaddr_in glvalue to sockaddr storage, which is an
// alignment/type-access issue (see the constructor and #4307).
sockaddr_in inet4{};
std::memcpy(&inet4, &m_data, sizeof(inet4));
const sockaddr_in &inet4 = reinterpret_cast<const sockaddr_in &>(m_data);
return ntohs(inet4.sin_port);
}

Expand All @@ -288,12 +279,12 @@ struct SocketAddr
std::string toString() const
{
std::ostringstream os;
const sockaddr &sa = reinterpret_cast<const sockaddr &>(m_data);

switch (m_data.sa_family)
switch (sa.sa_family)
{
case AF_INET: {
sockaddr_in inet4{};
std::memcpy(&inet4, &m_data, sizeof(inet4));
const sockaddr_in &inet4 = reinterpret_cast<const sockaddr_in &>(m_data);
u_long addr = ntohl(inet4.sin_addr.s_addr);
os << (addr >> 24) << '.' << ((addr >> 16) & 255) << '.' << ((addr >> 8) & 255) << '.'
<< (addr & 255);
Expand All @@ -302,7 +293,7 @@ struct SocketAddr
}

default:
os << "[?AF?" << m_data.sa_family << ']';
os << "[?AF?" << sa.sa_family << ']';
}
return os.str();
}
Expand All @@ -313,13 +304,12 @@ struct SocketAddr
// 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");

static_assert(sizeof(sockaddr_storage) >= sizeof(sockaddr_in),
"sockaddr_storage must be large enough to hold 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");
static_assert(sizeof(SocketAddr) == sizeof(sockaddr_storage),
"SocketAddr must add no storage beyond its sockaddr_storage");
/// <summary>
/// Encapsulation of a socket (non-exclusive ownership)
/// </summary>
Expand Down
Loading