From 1a3642cc741964f6339aeae9923bc0abbba36ce7 Mon Sep 17 00:00:00 2001 From: Matt Millett Date: Wed, 16 Sep 2026 13:53:35 -0400 Subject: [PATCH 1/2] When converting sockaddr_un to ntsa::Endpoint, treat sockaddr::sa_len as the authoritative end of the name, rather than relying on null termination --- groups/nts/ntsu/ntsu_socketutil.cpp | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/groups/nts/ntsu/ntsu_socketutil.cpp b/groups/nts/ntsu/ntsu_socketutil.cpp index a3db21fc..8d3107a2 100644 --- a/groups/nts/ntsu/ntsu_socketutil.cpp +++ b/groups/nts/ntsu/ntsu_socketutil.cpp @@ -937,11 +937,28 @@ ntsa::Error SocketUtil::Impl::convert(ntsa::Endpoint* endpoint, const bsl::size_t pathOffset = offsetof(struct sockaddr_un, sun_path); - if (socketAddressSize == pathOffset) { + const bsl::size_t addressSize = + static_cast(socketAddressSize); + + if (addressSize <= pathOffset) { localName->setUnnamed(); } else { + // The address length reported by the socket API is the + // authoritative extent of 'sun_path': the field is not + // guaranteed to be null-terminated (abstract names never are, + // and a peer may supply a truncated or unterminated path). + // Bound the scan by that length, clamped to the capacity of + // the field itself, so the scan never reads bytes outside the + // address. + + bsl::size_t pathSize = addressSize - pathOffset; + if (pathSize > sizeof(socketAddressLocal->sun_path)) { + pathSize = sizeof(socketAddressLocal->sun_path); + } + const char* begin = socketAddressLocal->sun_path; + const char* limit = begin + pathSize; if (*begin == 0) { localName->setAbstract(); @@ -949,7 +966,7 @@ ntsa::Error SocketUtil::Impl::convert(ntsa::Endpoint* endpoint, } const char* end = begin; - while (*end != 0) { + while (end != limit && *end != 0) { ++end; } @@ -957,7 +974,11 @@ ntsa::Error SocketUtil::Impl::convert(ntsa::Endpoint* endpoint, localName->setUnnamed(); } else { - localName->setValue(bslstl::StringRef(begin, end)); + ntsa::Error error = + localName->setValue(bslstl::StringRef(begin, end)); + if (error) { + return error; + } } } } From 4efce717bdcdc18a147b3b57c201a43f6d05d0d4 Mon Sep 17 00:00:00 2001 From: Matt Millett Date: Mon, 12 Oct 2026 11:29:16 -0400 Subject: [PATCH 2/2] Add tests for decoding Unix domain names that are not NUL terminator and contain embedded NULs --- groups/nts/ntsu/ntsu_socketutil.h | 6 +- groups/nts/ntsu/ntsu_socketutil.t.cpp | 187 ++++++++++++++++++++++---- 2 files changed, 169 insertions(+), 24 deletions(-) diff --git a/groups/nts/ntsu/ntsu_socketutil.h b/groups/nts/ntsu/ntsu_socketutil.h index c7f24c81..c1bfe5a8 100644 --- a/groups/nts/ntsu/ntsu_socketutil.h +++ b/groups/nts/ntsu/ntsu_socketutil.h @@ -723,7 +723,11 @@ struct SocketUtil { /// Load into the specified 'endpoint' the conversion of the specified /// 'socketAddress' having the specified 'socketAddressSize'. Return the - /// error. + /// error. Note a 'socketAddress' indicating a Unix domain name with an + /// embedded NUL is successfully decoded but the resulting 'endpoint' local + /// name only contains characters up to the first NUL character, not + /// including the initital NUL character, if any, that indicates the Unix + /// domain name is in the abstract namespace. static ntsa::Error decodeEndpoint(ntsa::Endpoint* endpoint, const void* socketAddress, bsl::size_t socketAddressSize); diff --git a/groups/nts/ntsu/ntsu_socketutil.t.cpp b/groups/nts/ntsu/ntsu_socketutil.t.cpp index e06757f8..b99ddcdb 100644 --- a/groups/nts/ntsu/ntsu_socketutil.t.cpp +++ b/groups/nts/ntsu/ntsu_socketutil.t.cpp @@ -50,6 +50,13 @@ BSLS_IDENT_RCSID(ntsu_socketutil_t_cpp, "$Id$ $CSID$") // clang-format on #endif +#if defined(BSLS_PLATFORM_OS_UNIX) +#include +#if NTSCFG_BUILD_WITH_TRANSPORT_PROTOCOL_LOCAL +#include +#endif +#endif + #if defined(BSLS_PLATFORM_OS_LINUX) #include #include @@ -459,6 +466,16 @@ class SocketUtilTest // Verify how the operating system reports the result of accepting a // socket whose peer has already closed the connection. static void verifyListenerSocketAcceptingClosedConnections(); + + // Verify endpoint decoding of a Unix domain socket name bounds the scan of + // 'sun_path' by the specified address size and does not depend on a null + // terminator. + static void verifyDecodeEndpointLocalUnterminated(); + + // Verify endpoint decoding of a Unix domain socket name with embedded NULs + // successfully returns an endpoint whose local name only contains + // characters up to the first embedded NUL. + static void verifyDecodeEndpointLocalEmbeddedNulls(); }; /// Describe the parameters of the test. @@ -479,7 +496,7 @@ class SocketUtilTest::Parameters /// Set the flag indicating that zero-copy transmissions should be /// performed according to the specified 'value'. void setZeroCopy(bool value); - + /// Return the flag indicating that timestamps for incoming data should be /// generated. bool timestampIncomingData() const; @@ -488,7 +505,7 @@ class SocketUtilTest::Parameters /// generated. bool timestampOutgoingData() const; - /// Return the flag indicating that zero-copy transmission should be + /// Return the flag indicating that zero-copy transmission should be /// performed. bool zeroCopy() const; @@ -567,10 +584,10 @@ bool SocketUtilTest::supportsTimestamping(ntsa::Handle socket) for (bsl::size_t i = 0; i < adapterList.size(); ++i) { const ntsa::Adapter& adapter = adapterList[i]; - if (adapter.ipv4Address().has_value() && - adapter.ipv4Address().value() == ntsa::Ipv4Address::loopback()) + if (adapter.ipv4Address().has_value() && + adapter.ipv4Address().value() == ntsa::Ipv4Address::loopback()) { - if (adapter.txSoftwareTimestamps() && + if (adapter.txSoftwareTimestamps() && adapter.rxSoftwareTimestamps()) { return true; @@ -716,7 +733,7 @@ void SocketUtilTest::executeStreamSocketTestParameters( if (ntsu::AdapterUtil::supportsTransportLoopback( ntsa::Transport::e_LOCAL_STREAM)) { - if (!parameters.timestampIncomingData() && + if (!parameters.timestampIncomingData() && !parameters.timestampOutgoingData() && !parameters.zeroCopy()) { @@ -749,7 +766,7 @@ void SocketUtilTest::executeStreamSocketTestParameters( continue; } - error = ntsu::SocketOptionUtil::setTimestampIncomingData(listener, + error = ntsu::SocketOptionUtil::setTimestampIncomingData(listener, true); NTSCFG_TEST_OK(error); } @@ -807,7 +824,7 @@ void SocketUtilTest::executeStreamSocketTestParameters( // Set the RX timestamping options for the client according to the // test parameters. Note that TX timestamping cannot be enabled on - // sockets before they are connected but must be explicitly set after + // sockets before they are connected but must be explicitly set after // they are connected. if (parameters.timestampIncomingData()) { @@ -815,7 +832,7 @@ void SocketUtilTest::executeStreamSocketTestParameters( continue; } - error = ntsu::SocketOptionUtil::setTimestampIncomingData(client, + error = ntsu::SocketOptionUtil::setTimestampIncomingData(client, true); NTSCFG_TEST_OK(error); } @@ -833,7 +850,7 @@ void SocketUtilTest::executeStreamSocketTestParameters( continue; } - error = ntsu::SocketOptionUtil::setTimestampOutgoingData(client, + error = ntsu::SocketOptionUtil::setTimestampOutgoingData(client, true); NTSCFG_TEST_OK(error); } @@ -862,14 +879,14 @@ void SocketUtilTest::executeStreamSocketTestParameters( ntsu::SocketUtil::Guard serverGuard(server); // Set the TX timestamping options for the server according to the - // test parameters. + // test parameters. if (parameters.timestampOutgoingData()) { if (!SocketUtilTest::supportsTimestamping(server)) { continue; } - error = ntsu::SocketOptionUtil::setTimestampOutgoingData(server, + error = ntsu::SocketOptionUtil::setTimestampOutgoingData(server, true); NTSCFG_TEST_OK(error); } @@ -918,13 +935,13 @@ void SocketUtilTest::executeStreamSocketTestParameters( if (parameters.zeroCopy()) { bool clientZeroCopy; - error = ntsu::SocketOptionUtil::getZeroCopy(&clientZeroCopy, + error = ntsu::SocketOptionUtil::getZeroCopy(&clientZeroCopy, client); NTSCFG_TEST_OK(error); NTSCFG_TEST_TRUE(clientZeroCopy); bool serverZeroCopy; - error = ntsu::SocketOptionUtil::getZeroCopy(&serverZeroCopy, + error = ntsu::SocketOptionUtil::getZeroCopy(&serverZeroCopy, server); NTSCFG_TEST_OK(error); NTSCFG_TEST_TRUE(serverZeroCopy); @@ -1037,7 +1054,7 @@ void SocketUtilTest::executeDatagramSocketTestParameters( if (ntsu::AdapterUtil::supportsTransportLoopback( ntsa::Transport::e_LOCAL_DATAGRAM)) { - if (!parameters.timestampIncomingData() && + if (!parameters.timestampIncomingData() && !parameters.timestampOutgoingData() && !parameters.zeroCopy()) { @@ -1068,7 +1085,7 @@ void SocketUtilTest::executeDatagramSocketTestParameters( continue; } - error = ntsu::SocketOptionUtil::setTimestampIncomingData(server, + error = ntsu::SocketOptionUtil::setTimestampIncomingData(server, true); NTSCFG_TEST_OK(error); } @@ -1078,7 +1095,7 @@ void SocketUtilTest::executeDatagramSocketTestParameters( continue; } - error = ntsu::SocketOptionUtil::setTimestampOutgoingData(server, + error = ntsu::SocketOptionUtil::setTimestampOutgoingData(server, true); NTSCFG_TEST_OK(error); } @@ -1149,7 +1166,7 @@ void SocketUtilTest::executeDatagramSocketTestParameters( continue; } - error = ntsu::SocketOptionUtil::setTimestampIncomingData(client, + error = ntsu::SocketOptionUtil::setTimestampIncomingData(client, true); NTSCFG_TEST_OK(error); } @@ -1159,7 +1176,7 @@ void SocketUtilTest::executeDatagramSocketTestParameters( continue; } - error = ntsu::SocketOptionUtil::setTimestampOutgoingData(client, + error = ntsu::SocketOptionUtil::setTimestampOutgoingData(client, true); NTSCFG_TEST_OK(error); } @@ -1244,13 +1261,13 @@ void SocketUtilTest::executeDatagramSocketTestParameters( if (parameters.zeroCopy()) { bool clientZeroCopy; - error = ntsu::SocketOptionUtil::getZeroCopy(&clientZeroCopy, + error = ntsu::SocketOptionUtil::getZeroCopy(&clientZeroCopy, client); NTSCFG_TEST_OK(error); NTSCFG_TEST_TRUE(clientZeroCopy); bool serverZeroCopy; - error = ntsu::SocketOptionUtil::getZeroCopy(&serverZeroCopy, + error = ntsu::SocketOptionUtil::getZeroCopy(&serverZeroCopy, server); NTSCFG_TEST_OK(error); NTSCFG_TEST_TRUE(serverZeroCopy); @@ -8551,7 +8568,7 @@ NTSCFG_TEST_FUNCTION( NTSCFG_TEST_OK(error); if (!SocketUtilTest::supportsTimestamping(client) || - !SocketUtilTest::supportsTimestamping(server)) + !SocketUtilTest::supportsTimestamping(server)) { ntsu::SocketUtil::close(client); ntsu::SocketUtil::close(server); @@ -8734,7 +8751,7 @@ NTSCFG_TEST_FUNCTION( parameters.setZeroCopy(true); SocketUtilTest::executeDatagramSocketTestParameters( - &SocketUtilTest::testDatagramSocketTxTimestampsAndZeroCopy, + &SocketUtilTest::testDatagramSocketTxTimestampsAndZeroCopy, parameters); #endif @@ -9091,5 +9108,129 @@ NTSCFG_TEST_FUNCTION(ntsu::SocketUtilTest::verifyIsSocket) } } +NTSCFG_TEST_FUNCTION( + ntsu::SocketUtilTest::verifyDecodeEndpointLocalUnterminated) +{ +#if defined(BSLS_PLATFORM_OS_UNIX) +#if NTSCFG_BUILD_WITH_TRANSPORT_PROTOCOL_LOCAL + // Concern: decodeEndpoint bounds the scan of 'sun_path' by the specified + // address size and does not depend on a null terminator. + + // Fill well past the end of the address with non-null bytes so a read + // beyond the stated length changes the result. Terminate at the very end + // so the old unbounded scan stays inside this buffer. + + const bsl::size_t pathOffset = offsetof(struct sockaddr_un, sun_path); + + union { + sockaddr_un un; + char raw[512]; + } address; + + ntsa::Endpoint endpoint; + ntsa::Error error; + + // Pathname, unterminated: "abc" followed by 'x' bytes. + bsl::memset(&address, 'x', sizeof address); + address.raw[sizeof address - 1] = 0; + address.un.sun_family = AF_UNIX; + bsl::memcpy(address.un.sun_path, "abc", 3); + + error = ntsu::SocketUtil::decodeEndpoint(&endpoint, + &address, + pathOffset + 3); + NTSCFG_TEST_OK(error); + NTSCFG_TEST_TRUE(endpoint.isLocal()); + NTSCFG_TEST_FALSE(endpoint.local().isAbstract()); + NTSCFG_TEST_EQ(endpoint.local().value(), "abc"); + +#if defined(BSLS_PLATFORM_OS_LINUX) + // Abstract: leading NUL, "abc", followed by 'x' bytes. + bsl::memset(&address, 'x', sizeof address); + address.raw[sizeof address - 1] = 0; + address.un.sun_family = AF_UNIX; + address.un.sun_path[0] = 0; + bsl::memcpy(address.un.sun_path + 1, "abc", 3); + + error = ntsu::SocketUtil::decodeEndpoint(&endpoint, + &address, + pathOffset + 4); + NTSCFG_TEST_OK(error); + NTSCFG_TEST_TRUE(endpoint.local().isAbstract()); + NTSCFG_TEST_EQ(endpoint.local().value(), "abc"); +#endif +#endif +#endif +} + +NTSCFG_TEST_FUNCTION( + ntsu::SocketUtilTest::verifyDecodeEndpointLocalEmbeddedNulls) +{ +#if defined(BSLS_PLATFORM_OS_UNIX) +#if NTSCFG_BUILD_WITH_TRANSPORT_PROTOCOL_LOCAL + // Concern: Endpoint decoding of a Unix domain socket names in the + // persistent namespace with embedded NULs successfully returns an endpoint + // whose local name only contains characters up to the first embedded NUL. + + { + ntsa::Endpoint endpoint; + ntsa::Error error; + + sockaddr_un un; + bsl::memset(&un, 0, sizeof un); + + un.sun_family = AF_UNIX; + un.sun_path[0] = 'a'; + un.sun_path[1] = 'b'; + un.sun_path[2] = 'c'; + un.sun_path[3] = 0; + un.sun_path[4] = 'x'; + + un.sun_len = sizeof(un.sun_family) + 5; + + error = ntsu::SocketUtil::decodeEndpoint( + &endpoint, + &un, + static_cast(un.sun_len)); + NTSCFG_TEST_OK(error); + NTSCFG_TEST_TRUE(endpoint.local().isPersistent()); + NTSCFG_TEST_EQ(endpoint.local().value(), "abc"); + } + +#if defined(BSLS_PLATFORM_OS_LINUX) + // Concern: Endpoint decoding of a Unix domain socket names in the + // abstract namespace with embedded NULs successfully returns an endpoint + // whose local name only contains characters up to the first embedded NUL, + // not including the first NUL which indicates the abstract namespace. + + { + ntsa::Endpoint endpoint; + ntsa::Error error; + + sockaddr_un un; + bsl::memset(&un, 0, sizeof un); + + un.sun_family = AF_UNIX; + un.sun_path[0] = 0; + un.sun_path[1] = 'a'; + un.sun_path[2] = 'b'; + un.sun_path[3] = 'c'; + un.sun_path[4] = 0; + un.sun_path[5] = 'x'; + + un.sun_len = sizeof(un.sun_family) + 6; + + error = ntsu::SocketUtil::decodeEndpoint( + &endpoint, + &un, + static_cast(un.sun_len)); + NTSCFG_TEST_OK(error); + NTSCFG_TEST_TRUE(endpoint.local().isPersistent()); + } +#endif +#endif +#endif +} + } // close namespace ntsu } // close namespace BloombergLP