[glib/wip/pwithnall/1954-dbus-keyring-handling: 6/6] gdbusauthmechanismsha1: Reduce syscalls from ensure_keyring_directory()

[Philip Withnall <pwithnall src gnome org>]

commit b3068f68602d740e75276da996abeddd661fbbcf
Author: Philip Withnall <withnall endlessm com>
Date:   Tue May 5 15:40:46 2020 +0100

    gdbusauthmechanismsha1: Reduce syscalls from ensure_keyring_directory()
    
    There’s no need to call `access()` and then `stat()` on the keyring
    directory to check that it exists, is a directory, and has the right
    permissions. Just call `stat()`.
    
    This eliminates one potential TOCTTOU race in this code.
    
    Signed-off-by: Philip Withnall <withnall endlessm com>
    
    Helps: #1954

 gio/gdbusauthmechanismsha1.c | 85 ++++++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 39 deletions(-)
---
diff --git a/gio/gdbusauthmechanismsha1.c b/gio/gdbusauthmechanismsha1.c
index 9385ed155..5e3e93d13 100644
--- a/gio/gdbusauthmechanismsha1.c
+++ b/gio/gdbusauthmechanismsha1.c
@@ -234,6 +234,9 @@ ensure_keyring_directory (GError **error)
 {
   gchar *path;
   const gchar *e;
+#ifdef G_OS_UNIX
+  struct stat statbuf;
+#endif
 
   g_return_val_if_fail (error == NULL || *error == NULL, NULL);
 
@@ -249,48 +252,54 @@ ensure_keyring_directory (GError **error)
                                NULL);
     }
 
-  if (g_file_test (path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR))
+#ifdef G_OS_UNIX
+  if (stat (path, &statbuf) != 0)
     {
-      if (g_getenv ("G_DBUS_COOKIE_SHA1_KEYRING_DIR_IGNORE_PERMISSION") == NULL)
+      int errsv = errno;
+
+      if (errsv != ENOENT)
         {
-#ifdef G_OS_UNIX
-          struct stat statbuf;
-          if (stat (path, &statbuf) != 0)
-            {
-              int errsv = errno;
-              g_set_error (error,
-                           G_IO_ERROR,
-                           g_io_error_from_errno (errsv),
-                           _("Error when getting information for directory “%s”: %s"),
-                           path,
-                           g_strerror (errsv));
-              g_free (path);
-              path = NULL;
-              goto out;
-            }
-          if ((statbuf.st_mode  & 0777) != 0700)
-            {
-              g_set_error (error,
-                           G_IO_ERROR,
-                           G_IO_ERROR_FAILED,
-                           _("Permissions on directory “%s” are malformed. Expected mode 0700, got 0%o"),
-                           path,
-                           (guint) (statbuf.st_mode & 0777));
-              g_free (path);
-              path = NULL;
-              goto out;
-            }
-#else
+          g_set_error (error,
+                       G_IO_ERROR,
+                       g_io_error_from_errno (errsv),
+                       _("Error when getting information for directory “%s”: %s"),
+                       path,
+                       g_strerror (errsv));
+          g_clear_pointer (&path, g_free);
+          return NULL;
+        }
+    }
+  else if (S_ISDIR (statbuf.st_mode))
+    {
+      if (g_getenv ("G_DBUS_COOKIE_SHA1_KEYRING_DIR_IGNORE_PERMISSION") == NULL &&
+          (statbuf.st_mode & 0777) != 0700)
+        {
+          g_set_error (error,
+                       G_IO_ERROR,
+                       G_IO_ERROR_FAILED,
+                       _("Permissions on directory “%s” are malformed. Expected mode 0700, got 0%o"),
+                       path,
+                       (guint) (statbuf.st_mode & 0777));
+          g_clear_pointer (&path, g_free);
+          return NULL;
+        }
+
+      return g_steal_pointer (&path);
+    }
+#else  /* if !G_OS_UNIX */
+  /* On non-Unix platforms, check that it exists as a directory, but don’t do
+   * permissions checks at the moment. */
+  if (g_file_test (path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR))
+    {
 #ifdef __GNUC__
 #pragma GCC diagnostic push
 #pragma GCC diagnostic warning "-Wcpp"
 #warning Please implement permission checking on this non-UNIX platform
 #pragma GCC diagnostic pop
-#endif
-#endif
-        }
-      goto out;
+#endif  /* __GNUC__ */
+      return g_steal_pointer (&path);
     }
+#endif  /* if !G_OS_UNIX */
 
   if (g_mkdir_with_parents (path, 0700) != 0)
     {
@@ -301,13 +310,11 @@ ensure_keyring_directory (GError **error)
                    _("Error creating directory “%s”: %s"),
                    path,
                    g_strerror (errsv));
-      g_free (path);
-      path = NULL;
-      goto out;
+      g_clear_pointer (&path, g_free);
+      return NULL;
     }
 
-out:
-  return path;
+  return g_steal_pointer (&path);
 }
 
 /* ---------------------------------------------------------------------------------------------------- */