Commit 4dbee3a3 authored by Ulrich Sibiller's avatar Ulrich Sibiller Committed by Mihai Moldovan

nxcomp: use new macro SAFE_FREE(ptr)

Should be used instead of free() calls and will clear the pointer after calling free(). This can prevent double-free or use-after-free errors.
parent 2814677a
...@@ -37,6 +37,8 @@ ...@@ -37,6 +37,8 @@
#include "NXalert.h" #include "NXalert.h"
#include "Misc.h"
ChannelEndPoint::ChannelEndPoint(const char *spec) ChannelEndPoint::ChannelEndPoint(const char *spec)
: defaultTCPPort_(0), defaultTCPInterface_(0), : defaultTCPPort_(0), defaultTCPInterface_(0),
defaultUnixPath_(NULL), spec_(NULL) { defaultUnixPath_(NULL), spec_(NULL) {
...@@ -54,17 +56,14 @@ ChannelEndPoint::~ChannelEndPoint() ...@@ -54,17 +56,14 @@ ChannelEndPoint::~ChannelEndPoint()
if(S_ISSOCK(st.st_mode)) if(S_ISSOCK(st.st_mode))
unlink(unixPath); unlink(unixPath);
} }
free(unixPath); SAFE_FREE(unixPath);
unixPath = NULL; SAFE_FREE(defaultUnixPath_);
free(defaultUnixPath_); SAFE_FREE(spec_);
defaultUnixPath_ = NULL;
free(spec_);
spec_ = NULL;
} }
void void
ChannelEndPoint::setSpec(const char *spec) { ChannelEndPoint::setSpec(const char *spec) {
free(spec_); SAFE_FREE(spec_);
if (spec && strlen(spec)) if (spec && strlen(spec))
{ {
...@@ -99,8 +98,7 @@ ChannelEndPoint::setSpec(const char *hostName, long port) { ...@@ -99,8 +98,7 @@ ChannelEndPoint::setSpec(const char *hostName, long port) {
isUnix_ = false; isUnix_ = false;
isTCP_ = false; isTCP_ = false;
free(spec_); SAFE_FREE(spec_);
spec_ = NULL;
if (hostName && strlen(hostName) && port >= 1) if (hostName && strlen(hostName) && port >= 1)
{ {
...@@ -145,9 +143,9 @@ ChannelEndPoint::getSpec(char **socketUri) const { ...@@ -145,9 +143,9 @@ ChannelEndPoint::getSpec(char **socketUri) const {
*socketUri = strdup(newSocketUri); *socketUri = strdup(newSocketUri);
} }
free(newSocketUri); SAFE_FREE(newSocketUri);
free(unixPath); SAFE_FREE(unixPath);
free(hostName); SAFE_FREE(hostName);
if (NULL != *socketUri) if (NULL != *socketUri)
return true; return true;
...@@ -168,7 +166,7 @@ ChannelEndPoint::setDefaultTCPInterface(int publicInterface) { ...@@ -168,7 +166,7 @@ ChannelEndPoint::setDefaultTCPInterface(int publicInterface) {
void void
ChannelEndPoint::setDefaultUnixPath(char *path) { ChannelEndPoint::setDefaultUnixPath(char *path) {
free(defaultUnixPath_); SAFE_FREE(defaultUnixPath_);
if (path && strlen(path)) if (path && strlen(path))
defaultUnixPath_ = strdup(path); defaultUnixPath_ = strdup(path);
...@@ -337,10 +335,10 @@ ChannelEndPoint &ChannelEndPoint::operator=(const ChannelEndPoint &other) { ...@@ -337,10 +335,10 @@ ChannelEndPoint &ChannelEndPoint::operator=(const ChannelEndPoint &other) {
defaultTCPInterface_ = other.defaultTCPInterface_; defaultTCPInterface_ = other.defaultTCPInterface_;
old = defaultUnixPath_; old = defaultUnixPath_;
defaultUnixPath_ = (other.defaultUnixPath_ ? strdup(other.defaultUnixPath_) : NULL); defaultUnixPath_ = (other.defaultUnixPath_ ? strdup(other.defaultUnixPath_) : NULL);
free(old); SAFE_FREE(old);
old = spec_; old = spec_;
spec_ = (other.spec_ ? strdup(other.spec_) : NULL); spec_ = (other.spec_ ? strdup(other.spec_) : NULL);
free(old); SAFE_FREE(old);
isUnix_ = getUnixPath(); isUnix_ = getUnixPath();
isTCP_ = getTCPHostAndPort(); isTCP_ = getTCPHostAndPort();
return *this; return *this;
...@@ -352,7 +350,7 @@ std::ostream& operator<<(std::ostream& os, const ChannelEndPoint& endPoint) { ...@@ -352,7 +350,7 @@ std::ostream& operator<<(std::ostream& os, const ChannelEndPoint& endPoint) {
if (endPoint.getSpec(&endPointSpec)) if (endPoint.getSpec(&endPointSpec))
{ {
os << endPointSpec; os << endPointSpec;
free(endPointSpec); SAFE_FREE(endPointSpec);
} }
else else
os << "(invalid)"; os << "(invalid)";
......
...@@ -3187,8 +3187,7 @@ int SetupProxyConnection() ...@@ -3187,8 +3187,7 @@ int SetupProxyConnection()
nxinfo << "Loop: listenSocket is "<< ( listenSocket.enabled() ? "enabled" : "disabled") << ". " nxinfo << "Loop: listenSocket is "<< ( listenSocket.enabled() ? "enabled" : "disabled") << ". "
<< "The socket URI is '"<< ( socketUri != NULL ? socketUri : "<unset>") << "'.\n" << std::flush; << "The socket URI is '"<< ( socketUri != NULL ? socketUri : "<unset>") << "'.\n" << std::flush;
free(socketUri); SAFE_FREE(socketUri);
socketUri = NULL;
if (WE_INITIATE_CONNECTION) if (WE_INITIATE_CONNECTION)
{ {
...@@ -3196,7 +3195,7 @@ int SetupProxyConnection() ...@@ -3196,7 +3195,7 @@ int SetupProxyConnection()
{ {
nxinfo << "Loop: Going to connect to '" << socketUri nxinfo << "Loop: Going to connect to '" << socketUri
<< "'.\n" << std::flush; << "'.\n" << std::flush;
free(socketUri); SAFE_FREE(socketUri);
proxyFD = ConnectToRemote(connectSocket); proxyFD = ConnectToRemote(connectSocket);
...@@ -3219,7 +3218,7 @@ int SetupProxyConnection() ...@@ -3219,7 +3218,7 @@ int SetupProxyConnection()
{ {
nxinfo << "Loop: Going to wait for connection at '" nxinfo << "Loop: Going to wait for connection at '"
<< socketUri << "'.\n" << std::flush; << socketUri << "'.\n" << std::flush;
free(socketUri); SAFE_FREE(socketUri);
proxyFD = WaitForRemote(listenSocket); proxyFD = WaitForRemote(listenSocket);
...@@ -4287,10 +4286,8 @@ int ListenConnection(ChannelEndPoint &endpoint, const char *label) ...@@ -4287,10 +4286,8 @@ int ListenConnection(ChannelEndPoint &endpoint, const char *label)
else if (endpoint.getTCPHostAndPort(&host, &port)) { else if (endpoint.getTCPHostAndPort(&host, &port)) {
result = ListenConnectionTCP(host, port, label); result = ListenConnectionTCP(host, port, label);
} }
free(unixPath); SAFE_FREE(unixPath);
unixPath = NULL; SAFE_FREE(host);
free(host);
host = NULL;
return result; return result;
} }
...@@ -6222,7 +6219,7 @@ int WaitForRemote(ChannelEndPoint &socketAddress) ...@@ -6222,7 +6219,7 @@ int WaitForRemote(ChannelEndPoint &socketAddress)
cerr << "Info" << ": Waiting for connection from " cerr << "Info" << ": Waiting for connection from "
<< hostLabel << " on socket '" << socketUri << hostLabel << " on socket '" << socketUri
<< "'.\n"; << "'.\n";
free(socketUri); SAFE_FREE(socketUri);
// //
// How many times to loop waiting for connections // How many times to loop waiting for connections
...@@ -6311,7 +6308,7 @@ int WaitForRemote(ChannelEndPoint &socketAddress) ...@@ -6311,7 +6308,7 @@ int WaitForRemote(ChannelEndPoint &socketAddress)
cerr << "Info" << ": Accepted connection from this host on Unix file socket '" cerr << "Info" << ": Accepted connection from this host on Unix file socket '"
<< unixPath << "'.\n"; << unixPath << "'.\n";
free(unixPath); SAFE_FREE(unixPath);
break; break;
} }
...@@ -6744,19 +6741,15 @@ int ConnectToRemote(ChannelEndPoint &socketAddress) ...@@ -6744,19 +6741,15 @@ int ConnectToRemote(ChannelEndPoint &socketAddress)
} }
} }
free(unixPath); SAFE_FREE(unixPath);
unixPath = NULL; SAFE_FREE(hostName);
free(hostName);
hostName = NULL;
return pFD; return pFD;
ConnectToRemoteError: ConnectToRemoteError:
free(unixPath); SAFE_FREE(unixPath);
unixPath = NULL; SAFE_FREE(hostName);
free(hostName);
hostName = NULL;
if (pFD != -1) if (pFD != -1)
{ {
...@@ -7953,7 +7946,7 @@ int ParseEnvironmentOptions(const char *env, int force) ...@@ -7953,7 +7946,7 @@ int ParseEnvironmentOptions(const char *env, int force)
cerr << "Error" << ": Refusing 'listen' parameter with 'connect' being '" cerr << "Error" << ": Refusing 'listen' parameter with 'connect' being '"
<< socketUri << "'.\n"; << socketUri << "'.\n";
free(socketUri); SAFE_FREE(socketUri);
return -1; return -1;
} }
...@@ -7981,7 +7974,7 @@ int ParseEnvironmentOptions(const char *env, int force) ...@@ -7981,7 +7974,7 @@ int ParseEnvironmentOptions(const char *env, int force)
cerr << "Error" << ": Refusing 'accept' parameter with 'connect' being '" cerr << "Error" << ": Refusing 'accept' parameter with 'connect' being '"
<< socketUri << "'.\n"; << socketUri << "'.\n";
free(socketUri); SAFE_FREE(socketUri);
return -1; return -1;
} }
......
...@@ -54,6 +54,9 @@ using namespace std; ...@@ -54,6 +54,9 @@ using namespace std;
#define EGET() (errno) #define EGET() (errno)
#define ESTR() strerror(errno) #define ESTR() strerror(errno)
// a free() macro that clears the ptr after free
#define SAFE_FREE(ptr) do { free(ptr); ptr = NULL; } while (0)
// //
// TCP port offset applied to NX port specification. // TCP port offset applied to NX port specification.
// //
......
...@@ -203,7 +203,7 @@ FILE *Popen(char * const parameters[], const char *type) ...@@ -203,7 +203,7 @@ FILE *Popen(char * const parameters[], const char *type)
if (pipe(pdes) < 0) if (pipe(pdes) < 0)
{ {
free(cur); SAFE_FREE(cur);
return NULL; return NULL;
} }
...@@ -237,7 +237,7 @@ FILE *Popen(char * const parameters[], const char *type) ...@@ -237,7 +237,7 @@ FILE *Popen(char * const parameters[], const char *type)
close(pdes[0]); close(pdes[0]);
close(pdes[1]); close(pdes[1]);
free(cur); SAFE_FREE(cur);
return NULL; return NULL;
} }
...@@ -420,7 +420,7 @@ int Pclose(FILE *iop) ...@@ -420,7 +420,7 @@ int Pclose(FILE *iop)
last -> next = cur -> next; last -> next = cur -> next;
} }
free(cur); SAFE_FREE(cur);
// //
// Child has finished and we called the // Child has finished and we called the
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment