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.

4 files changed, 33 insertions, 39 deletions

diff --git a/nxcomp/src/ChannelEndPoint.cpp b/nxcomp/src/ChannelEndPoint.cpp
index 843bf2b35..fb8549f6a 100644
--- a/nxcomp/src/ChannelEndPoint.cpp
+++ b/nxcomp/src/ChannelEndPoint.cpp
@@ -37,6 +37,8 @@
 
 #include "NXalert.h"
 
+#include "Misc.h"
+
 ChannelEndPoint::ChannelEndPoint(const char *spec)
   : defaultTCPPort_(0), defaultTCPInterface_(0),
     defaultUnixPath_(NULL), spec_(NULL) {
@@ -54,17 +56,14 @@ ChannelEndPoint::~ChannelEndPoint()
     if(S_ISSOCK(st.st_mode))
       unlink(unixPath);
   }
-  free(unixPath);
-  unixPath = NULL;
-  free(defaultUnixPath_);
-  defaultUnixPath_ = NULL;
-  free(spec_);
-  spec_ = NULL;
+  SAFE_FREE(unixPath);
+  SAFE_FREE(defaultUnixPath_);
+  SAFE_FREE(spec_);
 }
 
 void
 ChannelEndPoint::setSpec(const char *spec) {
-  free(spec_);
+  SAFE_FREE(spec_);
 
   if (spec && strlen(spec))
   {
@@ -99,8 +98,7 @@ ChannelEndPoint::setSpec(const char *hostName, long port) {
   isUnix_ = false;
   isTCP_ = false;
 
-  free(spec_);
-  spec_ = NULL;
+  SAFE_FREE(spec_);
 
   if (hostName && strlen(hostName) && port >= 1)
   {
@@ -145,9 +143,9 @@ ChannelEndPoint::getSpec(char **socketUri) const {
       *socketUri = strdup(newSocketUri);
   }
 
-  free(newSocketUri);
-  free(unixPath);
-  free(hostName);
+  SAFE_FREE(newSocketUri);
+  SAFE_FREE(unixPath);
+  SAFE_FREE(hostName);
 
   if (NULL != *socketUri)
     return true;
@@ -168,7 +166,7 @@ ChannelEndPoint::setDefaultTCPInterface(int publicInterface) {
 
 void
 ChannelEndPoint::setDefaultUnixPath(char *path) {
-  free(defaultUnixPath_);
+  SAFE_FREE(defaultUnixPath_);
 
   if (path && strlen(path))
     defaultUnixPath_ = strdup(path);
@@ -337,10 +335,10 @@ ChannelEndPoint &ChannelEndPoint::operator=(const ChannelEndPoint &other) {
   defaultTCPInterface_ = other.defaultTCPInterface_;
   old = defaultUnixPath_;
   defaultUnixPath_ = (other.defaultUnixPath_ ? strdup(other.defaultUnixPath_) : NULL);
-  free(old);
+  SAFE_FREE(old);
   old = spec_;
   spec_ = (other.spec_ ? strdup(other.spec_) : NULL);
-  free(old);
+  SAFE_FREE(old);
   isUnix_ = getUnixPath();
   isTCP_ = getTCPHostAndPort();
   return *this;
@@ -352,7 +350,7 @@ std::ostream& operator<<(std::ostream& os, const ChannelEndPoint& endPoint) {
     if (endPoint.getSpec(&endPointSpec))
     {
       os << endPointSpec;
-      free(endPointSpec);
+      SAFE_FREE(endPointSpec);
     }
     else
       os << "(invalid)";
diff --git a/nxcomp/src/Loop.cpp b/nxcomp/src/Loop.cpp
index ca9e5ed08..baad17699 100644
--- a/nxcomp/src/Loop.cpp
+++ b/nxcomp/src/Loop.cpp
@@ -3187,8 +3187,7 @@ int SetupProxyConnection()
     nxinfo << "Loop: listenSocket is "<< ( listenSocket.enabled() ? "enabled" : "disabled") << ". "
            << "The socket URI is '"<< ( socketUri != NULL ? socketUri : "<unset>") << "'.\n" << std::flush;
 
-    free(socketUri);
-    socketUri = NULL;
+    SAFE_FREE(socketUri);
 
     if (WE_INITIATE_CONNECTION)
     {
@@ -3196,7 +3195,7 @@ int SetupProxyConnection()
       {
         nxinfo << "Loop: Going to connect to '" << socketUri
                << "'.\n" << std::flush;
-        free(socketUri);
+        SAFE_FREE(socketUri);
 
         proxyFD = ConnectToRemote(connectSocket);
 
@@ -3219,7 +3218,7 @@ int SetupProxyConnection()
       {
         nxinfo << "Loop: Going to wait for connection at '"
                << socketUri << "'.\n" << std::flush;
-        free(socketUri);
+        SAFE_FREE(socketUri);
 
         proxyFD = WaitForRemote(listenSocket);
 
@@ -4287,10 +4286,8 @@ int ListenConnection(ChannelEndPoint &endpoint, const char *label)
   else if (endpoint.getTCPHostAndPort(&host, &port)) {
     result = ListenConnectionTCP(host, port, label);
   }
-  free(unixPath);
-  unixPath = NULL;
-  free(host);
-  host = NULL;
+  SAFE_FREE(unixPath);
+  SAFE_FREE(host);
   return result;
 }
 
@@ -6222,7 +6219,7 @@ int WaitForRemote(ChannelEndPoint &socketAddress)
   cerr << "Info" << ": Waiting for connection from "
        << hostLabel << " on socket '" << socketUri
        << "'.\n";
-  free(socketUri);
+  SAFE_FREE(socketUri);
 
   //
   // How many times to loop waiting for connections
@@ -6311,7 +6308,7 @@ int WaitForRemote(ChannelEndPoint &socketAddress)
 
         cerr << "Info" << ": Accepted connection from this host on Unix file socket '"
              << unixPath << "'.\n";
-        free(unixPath);
+        SAFE_FREE(unixPath);
 
         break;
       }
@@ -6744,19 +6741,15 @@ int ConnectToRemote(ChannelEndPoint &socketAddress)
     }
   }
 
-  free(unixPath);
-  unixPath = NULL;
-  free(hostName);
-  hostName = NULL;
+  SAFE_FREE(unixPath);
+  SAFE_FREE(hostName);
 
   return pFD;
 
 ConnectToRemoteError:
 
-  free(unixPath);
-  unixPath = NULL;
-  free(hostName);
-  hostName = NULL;
+  SAFE_FREE(unixPath);
+  SAFE_FREE(hostName);
 
   if (pFD != -1)
   {
@@ -7953,7 +7946,7 @@ int ParseEnvironmentOptions(const char *env, int force)
         cerr << "Error" << ": Refusing 'listen' parameter with 'connect' being '"
              << socketUri << "'.\n";
 
-        free(socketUri);
+        SAFE_FREE(socketUri);
         return -1;
       }
 
@@ -7981,7 +7974,7 @@ int ParseEnvironmentOptions(const char *env, int force)
         cerr << "Error" << ": Refusing 'accept' parameter with 'connect' being '"
              << socketUri << "'.\n";
 
-	free(socketUri);
+        SAFE_FREE(socketUri);
         return -1;
       }
 
diff --git a/nxcomp/src/Misc.h b/nxcomp/src/Misc.h
index 997630137..7808c34c2 100644
--- a/nxcomp/src/Misc.h
+++ b/nxcomp/src/Misc.h
@@ -54,6 +54,9 @@ using namespace std;
 #define EGET()   (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.
 //
diff --git a/nxcomp/src/Pipe.cpp b/nxcomp/src/Pipe.cpp
index 4fa149412..228c556ae 100644
--- a/nxcomp/src/Pipe.cpp
+++ b/nxcomp/src/Pipe.cpp
@@ -203,7 +203,7 @@ FILE *Popen(char * const parameters[], const char *type)
 
   if (pipe(pdes) < 0)
   {
-    free(cur);
+    SAFE_FREE(cur);
 
     return NULL;
   }
@@ -237,7 +237,7 @@ FILE *Popen(char * const parameters[], const char *type)
       close(pdes[0]);
       close(pdes[1]);
 
-      free(cur);
+      SAFE_FREE(cur);
 
       return NULL;
     }
@@ -420,7 +420,7 @@ int Pclose(FILE *iop)
     last -> next = cur -> next;
   }
 
-  free(cur);
+  SAFE_FREE(cur);
 
   //
   // Child has finished and we called the