open-telemetry / open-telemetry/opentelemetry-cpp
[BUG] SocketAddr(u_long, int) performs misaligned sockaddr_in access and truncates out-of-range ports
Nobody has claimed this yet.
- 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:
-1→6553565536→099999→34463
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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