Merge branch 'fix-issue-365' into 'master'

Fix memory leaks in _dbus_get_install_root_as_hash in case of OOM condition

Closes #365

See merge request dbus/dbus!239

1 files changed, 160 insertions, 54 deletions

diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
index 43350cc7..9d9552d0 100644
--- a/dbus/dbus-sysdeps-win.c
+++ b/dbus/dbus-sysdeps-win.c
@@ -2934,78 +2934,151 @@ static const char *cDBusDaemonMutex = "DBusDaemonMutex";
 // named shm for dbus adress info (per user)
 static const char *cDBusDaemonAddressInfo = "DBusDaemonAddressInfo";
 
+/**
+ * Return the hash of the installation root directory, which can be
+ * used to construct a per-installation-root scope for autolaunching
+ *
+ * If the installation root directory could not be
+ * determined, the returned length is set to zero.
+ *
+ * @param out initialized DBusString instance to return hash string
+ * @returns #FALSE on OOM, #TRUE if not OOM
+ */
 static dbus_bool_t
-_dbus_get_install_root_as_hash(DBusString *out)
+_dbus_get_install_root_as_hash (DBusString *out)
 {
-    DBusString install_path;
+  DBusString install_path;
+  dbus_bool_t retval = FALSE;
+  _dbus_assert (out != NULL);
 
-    _dbus_string_init(&install_path);
+  if (!_dbus_string_init (&install_path))
+    return FALSE;
 
-    if (!_dbus_get_install_root (&install_path) ||
-        _dbus_string_get_length (&install_path) == 0)
-        return FALSE;
+  if (!_dbus_get_install_root (&install_path))
+    goto out;
+
+  /* the install path can't be determined */
+  if (_dbus_string_get_length (&install_path) == 0)
+    {
+      _dbus_string_set_length (out, 0);
+      retval = TRUE;
+      goto out;
+    }
 
-    _dbus_string_init(out);
-    _dbus_string_tolower_ascii(&install_path,0,_dbus_string_get_length(&install_path));
+  _dbus_string_tolower_ascii (&install_path, 0, _dbus_string_get_length (&install_path));
 
-    if (!_dbus_sha_compute (&install_path, out))
-        return FALSE;
+  if (!_dbus_sha_compute (&install_path, out))
+    goto out;
 
-    return TRUE;
+  retval = TRUE;
+
+out:
+  _dbus_string_free (&install_path);
+  return retval;
 }
 
+/**
+ * Build a name from \p basestring and \p scope, and append it to \p out
+ *
+ * The name will be suitable for naming Windows objects such as mutexes
+ * and shared memory segments that need to be unique for each distinct
+ * \p scope, but shared between clients with the same \p scope.
+ *
+ * If \p scope has one of the special values recognised in autolaunch:
+ * addresses on Windows, substitute a unique string based on the scope
+ * (the username or the hash of the installation path) instead of the
+ * literal scope itself.
+ *
+ * With the '*install-path' \p scope the returned length can be zero,
+ * indicating that the name could not be determined.
+ *
+ * @param out initialized DBusString instance to return bus address
+ * @returns #FALSE on OOM, #TRUE if not OOM
+ */
 static dbus_bool_t
 _dbus_get_address_string (DBusString *out, const char *basestring, const char *scope)
 {
-  _dbus_string_init(out);
-  _dbus_string_append(out,basestring);
+  _dbus_assert (out != NULL);
 
-  if (!scope)
+  if (!scope || strlen (scope) == 0)
     {
-      return TRUE;
+      return _dbus_string_append (out, basestring);
     }
-  else if (strcmp(scope,"*install-path") == 0
+  else if (strcmp (scope, "*install-path") == 0
         // for 1.3 compatibility
-        || strcmp(scope,"install-path") == 0)
+        || strcmp (scope, "install-path") == 0)
     {
       DBusString temp;
-      if (!_dbus_get_install_root_as_hash(&temp))
+      dbus_bool_t retval = FALSE;
+
+      if (!_dbus_string_init (&temp))
+        return FALSE;
+
+      if (!_dbus_get_install_root_as_hash (&temp))
+        goto out;
+
+      if (_dbus_string_get_length (&temp) == 0)
         {
-          _dbus_string_free(out);
-           return FALSE;
+          _dbus_string_set_length (out, 0);
+          retval = TRUE;
+          goto out;
         }
-      _dbus_string_append(out,"-");
-      _dbus_string_append(out,_dbus_string_get_const_data(&temp));
-      _dbus_string_free(&temp);
+
+      if (!_dbus_string_append_printf (out, "%s-%s", basestring, _dbus_string_get_const_data (&temp)))
+        goto out;
+
+      retval = TRUE;
+out:
+      _dbus_string_free (&temp);
+      return retval;
     }
-  else if (strcmp(scope,"*user") == 0)
+  else if (strcmp (scope, "*user") == 0)
     {
-      _dbus_string_append(out,"-");
-      if (!_dbus_append_user_from_current_process(out))
-        {
-           _dbus_string_free(out);
-           return FALSE;
-        }
+      char *sid = NULL;
+      dbus_bool_t retval;
+
+      if (!_dbus_getsid (&sid, _dbus_getpid()))
+        return FALSE;
+
+      retval = _dbus_string_append_printf (out, "%s-%s", basestring, sid);
+
+      LocalFree(sid);
+
+      return retval;
     }
-  else if (strlen(scope) > 0)
+  else /* strlen(scope) > 0 */
     {
-      _dbus_string_append(out,"-");
-      _dbus_string_append(out,scope);
-      return TRUE;
+      return _dbus_string_append_printf (out, "%s-%s", basestring, scope);
     }
-  return TRUE;
 }
 
+/**
+ * Return name of shared memory segment constructed from the autolaunch scope \p scope
+ *
+ * See @ref _dbus_get_address_string for further usage information.
+ *
+ * @param out initialized DBusString instance to return shared memory segment name
+ * @returns #FALSE on OOM, #TRUE if not OOM
+ */
 static dbus_bool_t
 _dbus_get_shm_name (DBusString *out,const char *scope)
 {
-  return _dbus_get_address_string (out,cDBusDaemonAddressInfo,scope);
+  return _dbus_get_address_string (out, cDBusDaemonAddressInfo, scope);
 }
 
+/**
+ * Return mutex name for scope \p scope in \p out
+ *
+ * See @ref _dbus_get_address_string for further usage information.
+ *
+ * @param out initialized DBusString instance to return mutex name
+ * @param scope scope for the requested mutex name
+ * @returns #FALSE on OOM, #TRUE if not OOM
+ */
 static dbus_bool_t
-_dbus_get_mutex_name (DBusString *out,const char *scope)
+_dbus_get_mutex_name (DBusString *out, const char *scope)
 {
-  return _dbus_get_address_string (out,cDBusDaemonMutex,scope);
+  return _dbus_get_address_string (out, cDBusDaemonMutex, scope);
 }
 
 dbus_bool_t
@@ -3014,8 +3087,13 @@ _dbus_daemon_is_session_bus_address_published (const char *scope)
   HANDLE lock;
   DBusString mutex_name;
 
+  if (!_dbus_string_init (&mutex_name))
+    return FALSE;
+
   _dbus_verbose ("scope:%s\n", scope);
-  if (!_dbus_get_mutex_name(&mutex_name,scope))
+  if (!_dbus_get_mutex_name (&mutex_name, scope) ||
+      /* not determinable */
+      _dbus_string_get_length (&mutex_name) == 0)
     {
       _dbus_string_free (&mutex_name);
       return FALSE;
@@ -3072,8 +3150,13 @@ _dbus_daemon_publish_session_bus_address (const char* address, const char *scope
 
   _dbus_assert (address);
 
+  if (!_dbus_string_init (&mutex_name))
+    return FALSE;
+
   _dbus_verbose ("address:%s scope:%s\n", address, scope);
-  if (!_dbus_get_mutex_name(&mutex_name,scope))
+  if (!_dbus_get_mutex_name (&mutex_name, scope) ||
+      /* not determinable */
+      _dbus_string_get_length (&mutex_name) == 0)
     {
       _dbus_string_free (&mutex_name);
       return FALSE;
@@ -3096,7 +3179,15 @@ _dbus_daemon_publish_session_bus_address (const char* address, const char *scope
       return FALSE;
     }
 
-  if (!_dbus_get_shm_name(&shm_name,scope))
+  if (!_dbus_string_init (&shm_name))
+    {
+      _dbus_global_unlock (lock);
+      return FALSE;
+    }
+
+  if (!_dbus_get_shm_name (&shm_name, scope) ||
+      /* not determinable */
+      _dbus_string_get_length (&shm_name) == 0)
     {
       _dbus_string_free (&shm_name);
       _dbus_global_unlock (lock);
@@ -3106,16 +3197,16 @@ _dbus_daemon_publish_session_bus_address (const char* address, const char *scope
   // create shm
   len = strlen (address) + 1;
 
-  hDBusSharedMem = CreateFileMappingA( INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE,
+  hDBusSharedMem = CreateFileMappingA ( INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE,
                                        len >> 32, len & 0xffffffffu,
-                                       _dbus_string_get_const_data(&shm_name) );
+                                       _dbus_string_get_const_data (&shm_name) );
   _dbus_assert (hDBusSharedMem);
 
   shared_addr = MapViewOfFile (hDBusSharedMem, FILE_MAP_WRITE, 0, 0, 0);
 
   _dbus_assert (shared_addr);
 
-  strcpy( shared_addr, address);
+  strcpy(shared_addr, address);
 
   // cleanup
   UnmapViewOfFile (shared_addr);
@@ -3175,7 +3266,7 @@ _dbus_get_autolaunch_shm (DBusString *address, DBusString *shm_name)
   // read shm
   for(i=0;i<20;++i) {
       // we know that dbus-daemon is available, so we wait until shm is available
-      sharedMem = OpenFileMappingA( FILE_MAP_READ, FALSE, _dbus_string_get_const_data(shm_name));
+      sharedMem = OpenFileMappingA (FILE_MAP_READ, FALSE, _dbus_string_get_const_data (shm_name));
       if (sharedMem == 0)
           Sleep (100);
       if ( sharedMem != 0)
@@ -3210,7 +3301,12 @@ _dbus_daemon_already_runs (DBusString *address, DBusString *shm_name, const char
   DBusString mutex_name;
   dbus_bool_t bRet = TRUE;
 
-  if (!_dbus_get_mutex_name (&mutex_name,scope))
+  if (!_dbus_string_init (&mutex_name))
+    return FALSE;
+
+  if (!_dbus_get_mutex_name (&mutex_name,scope) ||
+      /* not determinable */
+      _dbus_string_get_length (&mutex_name) == 0)
     {
       _dbus_string_free (&mutex_name);
       return FALSE;
@@ -3220,7 +3316,7 @@ _dbus_daemon_already_runs (DBusString *address, DBusString *shm_name, const char
   lock = _dbus_global_lock (cUniqueDBusInitMutex);
 
   // do checks
-  daemon = CreateMutexA (NULL, FALSE, _dbus_string_get_const_data(&mutex_name));
+  daemon = CreateMutexA (NULL, FALSE, _dbus_string_get_const_data (&mutex_name));
   if(WaitForSingleObject (daemon, 10) != WAIT_TIMEOUT)
     {
       ReleaseMutex (daemon);
@@ -3235,7 +3331,7 @@ _dbus_daemon_already_runs (DBusString *address, DBusString *shm_name, const char
   bRet = _dbus_get_autolaunch_shm (address, shm_name);
 
   // cleanup
-  CloseHandle  (daemon);
+  CloseHandle (daemon);
 
   _dbus_global_unlock (lock);
   _dbus_string_free (&mutex_name);
@@ -3244,10 +3340,11 @@ _dbus_daemon_already_runs (DBusString *address, DBusString *shm_name, const char
 }
 
 dbus_bool_t
-_dbus_get_autolaunch_address (const char *scope, DBusString *address,
+_dbus_get_autolaunch_address (const char *scope,
+                              DBusString *address,
                               DBusError *error)
 {
-  HANDLE mutex;
+  HANDLE mutex = NULL;
   STARTUPINFOA si;
   PROCESS_INFORMATION pi;
   dbus_bool_t retval = FALSE;
@@ -3259,17 +3356,25 @@ _dbus_get_autolaunch_address (const char *scope, DBusString *address,
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
-  if (!_dbus_get_shm_name (&shm_name,scope))
+  if (!_dbus_string_init (&shm_name))
     {
-      dbus_set_error_const (error, DBUS_ERROR_FAILED, "could not determine shm name");
+      _DBUS_SET_OOM(error);
       return FALSE;
     }
 
+  if (!_dbus_get_shm_name (&shm_name, scope) ||
+      /* not determinable */
+      _dbus_string_get_length (&shm_name) == 0)
+    {
+      dbus_set_error_const (error, DBUS_ERROR_FAILED, "could not determine shm name");
+      goto out;
+    }
+
   mutex = _dbus_global_lock (cDBusAutolaunchMutex);
 
   if (_dbus_daemon_already_runs (address, &shm_name, scope))
     {
-      _dbus_verbose ( "found running dbus daemon for scope '%s' at %s\n",
+      _dbus_verbose ("found running dbus daemon for scope '%s' at %s\n",
                      scope ? scope : "", _dbus_string_get_const_data (&shm_name) );
       retval = TRUE;
       goto out;
@@ -3349,7 +3454,8 @@ _dbus_get_autolaunch_address (const char *scope, DBusString *address,
 
 out:
   _DBUS_ASSERT_ERROR_XOR_BOOL (error, retval);
-  _dbus_global_unlock (mutex);
+  if (mutex != NULL)
+    _dbus_global_unlock (mutex);
   _dbus_string_free (&shm_name);
   _dbus_string_free (&dbus_args);