This is an automated email from the ASF dual-hosted git repository.
moonchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 1f34fdafd9 Reject over-long unix socket paths in server_ports (#13356)
1f34fdafd9 is described below
commit 1f34fdafd96f0506edc3bd18098fc54c0800345a
Author: Mo Chen <[email protected]>
AuthorDate: Tue Jul 14 09:14:27 2026 -0500
Reject over-long unix socket paths in server_ports (#13356)
* Reject over-long unix socket paths in server_ports
A path longer than sun_path (108 bytes including the terminator) was
silently truncated, and left unterminated, since strncpy writes no
terminator when it truncates, so ATS bound a listener on the wrong
filesystem path with nothing telling the operator the configured path
was too long.
Reject the path at configuration parse with a Warning naming the
limit, and make UnAddr's string constructors always null-terminate.
---
include/tscore/ink_inet.h | 22 +++++++++++++-----
src/records/RecHttp.cc | 7 ++++++
src/records/unit_tests/test_RecHttp.cc | 41 ++++++++++++++++++++++++++++++++++
src/tscore/unit_tests/test_ink_inet.cc | 32 ++++++++++++++++++++++++++
4 files changed, 96 insertions(+), 6 deletions(-)
diff --git a/include/tscore/ink_inet.h b/include/tscore/ink_inet.h
index 18319b1353..d0cc243364 100644
--- a/include/tscore/ink_inet.h
+++ b/include/tscore/ink_inet.h
@@ -34,6 +34,7 @@
#include "tscore/ink_memory.h"
#include "tscore/ink_apidefs.h"
+#include "tscore/ink_string.h"
#include "swoc/bwf_fwd.h"
#if !TS_HAS_IN6_IS_ADDR_UNSPECIFIED
@@ -1610,16 +1611,21 @@ struct UnAddr {
UnAddr() { _path[0] = 0; }
- UnAddr(self const &addr) { strncpy(_path, addr._path, TS_UNIX_SIZE); }
- explicit UnAddr(const char *path) { strncpy(_path, path, TS_UNIX_SIZE - 1); }
- explicit UnAddr(const std::string &path) { strncpy(_path, path.c_str(),
TS_UNIX_SIZE); }
+ UnAddr(self const &addr) { ink_strlcpy(_path, addr._path, TS_UNIX_SIZE); }
+ explicit UnAddr(const char *path) { ink_strlcpy(_path, path, TS_UNIX_SIZE); }
+ explicit UnAddr(const std::string &path) { ink_strlcpy(_path, path.c_str(),
TS_UNIX_SIZE); }
explicit UnAddr(sockaddr const *addr) { this->assign(addr); }
explicit UnAddr(sockaddr_un const *addr) {
this->assign(ats_ip_sa_cast(addr)); }
/// Construct from @c IpEndpoint.
explicit UnAddr(IpEndpoint const &addr) { this->assign(&addr.sa); }
/// Construct from @c IpEndpoint.
- explicit UnAddr(IpEndpoint const *addr) { this->assign(&addr->sa); }
+ explicit UnAddr(IpEndpoint const *addr) : UnAddr()
+ {
+ if (addr) {
+ this->assign(&addr->sa);
+ }
+ }
/// Assign sockaddr storage.
self &assign(sockaddr const *addr);
@@ -1639,7 +1645,7 @@ struct UnAddr {
operator=(self const &addr)
{
if (this != &addr) {
- strncpy(_path, addr._path, TS_UNIX_SIZE);
+ ink_strlcpy(_path, addr._path, TS_UNIX_SIZE);
}
return *this;
}
@@ -1651,7 +1657,11 @@ inline UnAddr &
UnAddr::assign(sockaddr const *addr)
{
if (addr) {
- strncpy(_path, ats_unix_cast(addr)->sun_path, TS_UNIX_SIZE);
+ // A kernel sockaddr_un may carry a full, unterminated sun_path.
+ strncpy(_path, ats_unix_cast(addr)->sun_path, TS_UNIX_SIZE - 1);
+ _path[TS_UNIX_SIZE - 1] = '\0';
+ } else {
+ _path[0] = '\0';
}
return *this;
}
diff --git a/src/records/RecHttp.cc b/src/records/RecHttp.cc
index 57cb822c99..6feade5dd1 100644
--- a/src/records/RecHttp.cc
+++ b/src/records/RecHttp.cc
@@ -368,6 +368,13 @@ HttpProxyPort::processOptions(const char *opts)
for (auto item : values) {
if (item[0] == '/') {
+ if (size_t len = strlen(item); len >= TS_UNIX_SIZE) {
+ // A longer path would be silently truncated and the listener bound to
the wrong
+ // filesystem path. Reject the whole descriptor so a later token (e.g.
a numeric
+ // port) can't silently turn this into an INET listener.
+ Warning("Unix path '%s' in port configuration '%s' is too long (%zu
bytes, max %zu)", item, opts, len, TS_UNIX_SIZE - 1);
+ return false;
+ }
m_family = AF_UNIX;
m_unix_path = UnAddr(item);
af_set_p = true;
diff --git a/src/records/unit_tests/test_RecHttp.cc
b/src/records/unit_tests/test_RecHttp.cc
index 9f0190d51f..4558631993 100644
--- a/src/records/unit_tests/test_RecHttp.cc
+++ b/src/records/unit_tests/test_RecHttp.cc
@@ -97,6 +97,47 @@ TEST_CASE("RecHttp", "[librecords][RecHttp]")
REQUIRE(view.find(":ssl") != TextView::npos);
REQUIRE(view.find(":proto") == TextView::npos); // it's default, should
not have this.
}
+
+ SECTION("unix-path")
+ {
+ HttpProxyPort::loadValue(ports, "/run/ats/uds.socket");
+ REQUIRE(ports.size() == 1);
+ REQUIRE(ports[0].m_family == AF_UNIX);
+ REQUIRE(std::string_view(ports[0].m_unix_path._path) ==
"/run/ats/uds.socket");
+ }
+
+ SECTION("unix-path-max")
+ {
+ // Exactly TS_UNIX_SIZE - 1 bytes: the longest path that fits sun_path
with its terminator.
+ std::string path = "/" + std::string(TS_UNIX_SIZE - 2, 'x');
+ HttpProxyPort::loadValue(ports, path.c_str());
+ REQUIRE(ports.size() == 1);
+ REQUIRE(ports[0].m_family == AF_UNIX);
+ REQUIRE(std::string_view(ports[0].m_unix_path._path) == path);
+ }
+
+ SECTION("unix-path-too-long")
+ {
+ // One byte past what sun_path can hold.
+ std::string path = "/" + std::string(TS_UNIX_SIZE - 1, 'x');
+ HttpProxyPort::loadValue(ports, path.c_str());
+ REQUIRE(ports.size() == 0);
+ // The reject itself, then loadValue's "No valid definition" for the
dropped descriptor.
+ REQUIRE(cdiag->messages.size() == 2);
+ REQUIRE(cdiag->messages[0].find("too long") != std::string::npos);
+ }
+
+ SECTION("unix-path-too-long-with-port")
+ {
+ // An over-long unix path followed by a numeric port must reject the whole
descriptor,
+ // not silently fall back to an INET port listener.
+ std::string path = "/" + std::string(TS_UNIX_SIZE - 1, 'x') + ":8080";
+ HttpProxyPort::loadValue(ports, path.c_str());
+ REQUIRE(ports.size() == 0);
+ // The reject itself, then loadValue's "No valid definition" for the
dropped descriptor.
+ REQUIRE(cdiag->messages.size() == 2);
+ REQUIRE(cdiag->messages[0].find("too long") != std::string::npos);
+ }
}
struct ConvertAlpnToWireFormatTestCase {
diff --git a/src/tscore/unit_tests/test_ink_inet.cc
b/src/tscore/unit_tests/test_ink_inet.cc
index b16234041a..0dd2304b10 100644
--- a/src/tscore/unit_tests/test_ink_inet.cc
+++ b/src/tscore/unit_tests/test_ink_inet.cc
@@ -280,4 +280,36 @@ TEST_CASE("ink_inet_unix", "[libts][inet][unix]")
#if HAVE_STRUCT_SOCKADDR_UN_SUN_LEN
REQUIRE(ep.sun.sun_len == SUN_LEN(&ep.sun));
#endif
+
+ // An over-long path must still yield a null terminated _path.
+ std::string long_path(TS_UNIX_SIZE + 10, 'y');
+ UnAddr from_cstr(long_path.c_str());
+ REQUIRE(from_cstr._path[TS_UNIX_SIZE - 1] == '\0');
+ REQUIRE(strlen(from_cstr._path) == TS_UNIX_SIZE - 1);
+ UnAddr from_string(long_path);
+ REQUIRE(from_string._path[TS_UNIX_SIZE - 1] == '\0');
+ REQUIRE(strlen(from_string._path) == TS_UNIX_SIZE - 1);
+
+ // A kernel sockaddr_un may carry a full, unterminated sun_path; assign()
and the copy
+ // paths must still yield a terminated _path.
+ IpEndpoint raw;
+ raw.sa.sa_family = AF_UNIX;
+ memset(raw.sun.sun_path, 'z', TS_UNIX_SIZE); // deliberately no terminator
+ UnAddr from_sockaddr(&raw.sa);
+ REQUIRE(from_sockaddr._path[TS_UNIX_SIZE - 1] == '\0');
+ REQUIRE(strlen(from_sockaddr._path) == TS_UNIX_SIZE - 1);
+ UnAddr copied(from_sockaddr);
+ REQUIRE(copied._path[TS_UNIX_SIZE - 1] == '\0');
+ UnAddr assigned;
+ assigned = from_sockaddr;
+ REQUIRE(assigned._path[TS_UNIX_SIZE - 1] == '\0');
+
+ // Constructing from a null address must leave _path an empty, terminated
string so the copy
+ // paths don't read uninitialized memory.
+ UnAddr from_null_sa{static_cast<sockaddr const *>(nullptr)};
+ REQUIRE(from_null_sa._path[0] == '\0');
+ UnAddr from_null_ep{static_cast<IpEndpoint const *>(nullptr)};
+ REQUIRE(from_null_ep._path[0] == '\0');
+ UnAddr copied_null(from_null_sa);
+ REQUIRE(copied_null._path[0] == '\0');
}
