open-telemetry / open-telemetry/opentelemetry-cpp

[BUG] SocketAddr(u_long, int) performs misaligned sockaddr_in access and truncates out-of-range ports

Open
#4,307 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

triage/accepted
Dominant language
C++
Stars
1.4k
Forks
632
Avg merge
1d 13h
Merged PRs (30d)
75

Description

Description

SocketTools::SocketAddr(u_long addr, int port) in ext/include/opentelemetry/ext/http/server/socket_tools.h has two defects:

1. Alignment / type-access UB. It binds a sockaddr_in & to the sockaddr m_data storage through a reinterpret_cast:

sockaddr_in &inet4 = reinterpret_cast<sockaddr_in &>(m_data);

sockaddr and sockaddr_in do not share alignment on all platforms (on Linux alignof(sockaddr) == 2 but alignof(sockaddr_in) == 4), so accessing the storage through a sockaddr_in glvalue is an alignment / strict-aliasing issue that UBSan reports.

2. Silent port truncation. port is an int cast to uint16_t with no range check:

inet4.sin_port = htons(static_cast<uint16_t>(port));

so out-of-range values wrap silently:

  • -165535
  • 655360
  • 9999934463
Impact

This constructor is on the live server path: it has an addListeningPort caller (unlike the SocketAddr(char const *) overload, which has no in-repo caller). The string-parsing overload was made safe in #4292 (it memcpys a local sockaddr_in in and out of the storage and parses a strict decimal port); this integer constructor still uses the old reinterpret_cast access and the unchecked port cast.

Suggested fix

Mirror #4292's approach: build a local sockaddr_in, memcpy it into m_data, and validate or clamp the port range (or document the caller's contract that the port is already in 0..65535).

Context

Split out of the ext/http correctness audit (#4287). #4292 deliberately scoped this out to keep that PR limited to the string parser.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in ext/include/opentelemetry/ext/http/server/socket_tools.h at SocketTools::SocketAddr(u_long addr, int port), then trace its addListeningPort caller. Compare the integer overload with the memcpy-based approach from #4292. Done means the constructor avoids misaligned sockaddr_in access and has defined behavior for ports outside 0..65535, with UBSan and relevant behavior checks passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.