From 6198e0376f9ce8130af3294fa284659f0055610d Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Wed, 27 Dec 2017 14:53:58 +0100 Subject: nxcomp: implement correct length handling for unix socket structs (partially) fixes ArcticaProject/nx-libs#612 --- nxcomp/src/Proxy.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) (limited to 'nxcomp/src/Proxy.cpp') diff --git a/nxcomp/src/Proxy.cpp b/nxcomp/src/Proxy.cpp index 437296f60..963ae3d75 100644 --- a/nxcomp/src/Proxy.cpp +++ b/nxcomp/src/Proxy.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -6294,19 +6295,12 @@ int Proxy::handleNewGenericConnectionFromProxyUnix(int channelId, T_channel_type serverAddrUnix.sun_family = AF_UNIX; - #ifdef __linux__ - const int serverAddrNameLength = 108; - #else - /* POSIX/SUS does not specify a length. - * BSD derivatives generally support 104 bytes, other systems may be more constrained. - * If you happen to run into such systems, extend this section with the appropriate limit. - */ - const int serverAddrNameLength = 104; - #endif - - strncpy(serverAddrUnix.sun_path, path, serverAddrNameLength); + // determine the maximum number of characters that fit into struct + // sockaddr_un's sun_path member + std::size_t serverAddrNameLength = + sizeof(struct sockaddr_un) - offsetof(struct sockaddr_un, sun_path); - *(serverAddrUnix.sun_path + serverAddrNameLength - 1) = '\0'; + snprintf(serverAddrUnix.sun_path, serverAddrNameLength, "%s", path); #ifdef TEST *logofs << "Proxy: Connecting to " << label << " server " -- cgit v1.2.3 From eae64c4a4282eb2b511ba11c6db51d00a3b49833 Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Wed, 27 Dec 2017 17:06:43 +0100 Subject: ChannelEndPoint.cpp: fix another memleak ==7689== 50 bytes in 5 blocks are definitely lost in loss record 1 of 2 ==7689== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==7689== by 0x54074D9: strndup (strndup.c:43) ==7689== by 0x4E7D803: ChannelEndPoint::getTCPHostAndPort(char**, long*) const (ChannelEndPoint.cpp:309) ==7689== by 0x4EC9D93: ConnectToRemote(ChannelEndPoint&) [clone .constprop.144] (Loop.cpp:6660) ==7689== by 0x4ECB94E: SetupProxyConnection() (Loop.cpp:3204) ==7689== by 0x4ECE824: handleNegotiationInLoop(int&, fd_set&, fd_set&, timeval&) [clone .isra.129] (Loop.cpp:14312) ==7689== by 0x4ED0F8A: NXTransPrepare (Loop.cpp:2575) ==7689== by 0x4ED1C35: NXTransContinue (Loop.cpp:1609) ==7689== by 0x4ED1D7B: WaitCleanup() (Loop.cpp:4440) ==7689== by 0x4ED2343: NXTransProxy (Loop.cpp:1234) ==7689== by 0x400B2A: main (Main.c:111) --- nxcomp/src/ChannelEndPoint.cpp | 6 ++++-- nxcomp/src/Loop.cpp | 2 ++ nxcomp/src/Proxy.cpp | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) (limited to 'nxcomp/src/Proxy.cpp') diff --git a/nxcomp/src/ChannelEndPoint.cpp b/nxcomp/src/ChannelEndPoint.cpp index 2abc0ee6a..de881835c 100644 --- a/nxcomp/src/ChannelEndPoint.cpp +++ b/nxcomp/src/ChannelEndPoint.cpp @@ -273,8 +273,10 @@ ChannelEndPoint::getTCPHostAndPort(char **host, long *port) const { char *h = NULL; ssize_t h_len; - if (host) *host = NULL; - if (port) *port = 0; + if (host) + *host = NULL; + if (port) + *port = 0; if (getPort(&p)) { h_len = 0; diff --git a/nxcomp/src/Loop.cpp b/nxcomp/src/Loop.cpp index 3ee094efb..ab7ea5a0f 100644 --- a/nxcomp/src/Loop.cpp +++ b/nxcomp/src/Loop.cpp @@ -6660,6 +6660,8 @@ int ConnectToRemote(ChannelEndPoint &socketAddress) << " in process with pid '" << getpid() << "'.\n" << std::flush; + SAFE_FREE(hostName); + if (socketAddress.getUnixPath(&unixPath)) result = PrepareProxyConnectionUnix(&unixPath, &connectTimeout, &pFD, &reason); else if (socketAddress.getTCPHostAndPort(&hostName, &portNum)) diff --git a/nxcomp/src/Proxy.cpp b/nxcomp/src/Proxy.cpp index 963ae3d75..71ea090e6 100644 --- a/nxcomp/src/Proxy.cpp +++ b/nxcomp/src/Proxy.cpp @@ -6123,7 +6123,7 @@ int Proxy::handleNewSlaveConnection(int clientFd) int Proxy::handleNewGenericConnectionFromProxy(int channelId, T_channel_type type, ChannelEndPoint &endPoint, const char *label) { - char *unixPath, *host; + char *unixPath, *host = NULL; long port; if (endPoint.getUnixPath(&unixPath)) { -- cgit v1.2.3 From ce293647d5a63726c05260ca0e0f65a50e604ebb Mon Sep 17 00:00:00 2001 From: Ulrich Sibiller Date: Wed, 27 Dec 2017 19:16:15 +0100 Subject: ChannelEndPoint.cpp: fix possible memleak in getUnixPath() --- nxcomp/src/ChannelEndPoint.cpp | 8 +++++--- nxcomp/src/Loop.cpp | 1 + nxcomp/src/Proxy.cpp | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) (limited to 'nxcomp/src/Proxy.cpp') diff --git a/nxcomp/src/ChannelEndPoint.cpp b/nxcomp/src/ChannelEndPoint.cpp index 443c0f290..7768df137 100644 --- a/nxcomp/src/ChannelEndPoint.cpp +++ b/nxcomp/src/ChannelEndPoint.cpp @@ -207,7 +207,10 @@ ChannelEndPoint::getPort(long *port) const { bool ChannelEndPoint::getUnixPath(char **unixPath) const { - if (unixPath) *unixPath = NULL; + if (unixPath) + *unixPath = NULL; + else + return false; long p; char *path = NULL; @@ -227,8 +230,7 @@ ChannelEndPoint::getUnixPath(char **unixPath) const { return false; } - if (unixPath) - *unixPath = strdup(path); + *unixPath = strdup(path); return true; } diff --git a/nxcomp/src/Loop.cpp b/nxcomp/src/Loop.cpp index ab7ea5a0f..1e770380f 100644 --- a/nxcomp/src/Loop.cpp +++ b/nxcomp/src/Loop.cpp @@ -6661,6 +6661,7 @@ int ConnectToRemote(ChannelEndPoint &socketAddress) << "'.\n" << std::flush; SAFE_FREE(hostName); + SAFE_FREE(unixPath); if (socketAddress.getUnixPath(&unixPath)) result = PrepareProxyConnectionUnix(&unixPath, &connectTimeout, &pFD, &reason); diff --git a/nxcomp/src/Proxy.cpp b/nxcomp/src/Proxy.cpp index 71ea090e6..7f72fae3f 100644 --- a/nxcomp/src/Proxy.cpp +++ b/nxcomp/src/Proxy.cpp @@ -6123,7 +6123,7 @@ int Proxy::handleNewSlaveConnection(int clientFd) int Proxy::handleNewGenericConnectionFromProxy(int channelId, T_channel_type type, ChannelEndPoint &endPoint, const char *label) { - char *unixPath, *host = NULL; + char *unixPath = NULL, *host = NULL; long port; if (endPoint.getUnixPath(&unixPath)) { -- cgit v1.2.3