gfileutils: Fix potential integer overflow in g_get_current_dir()

In practice, this will never happen.

If `getcwd()` returns `ERANGE` whenever the working directory is ≥
`PATH_MAX`, though, the previous implementation of the loop would run
until `max_len == G_MAXULONG`, and would then overflow when adding `1`
to it for a nul terminator in the allocation.

Avoid that problem by always keeping `buffer_len` as a power of two, and
relying on `getcwd()` to write a nul terminator within the buffer space
it’s given. It seems to do this on all platforms we care about, because
the previous version of the code never explicitly wrote a nul terminator
anywhere.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #98

1 files changed, 14 insertions, 10 deletions

diff --git a/glib/gfileutils.c b/glib/gfileutils.c
index 4ed8171cc..9646c696e 100644
--- a/glib/gfileutils.c
+++ b/glib/gfileutils.c
@@ -2940,7 +2940,7 @@ g_get_current_dir (void)
   const gchar *pwd;
   gchar *buffer = NULL;
   gchar *dir = NULL;
-  static gulong max_len = 0;
+  static gsize buffer_size = 0;
   struct stat pwdbuf, dotbuf;
 
   pwd = g_getenv ("PWD");
@@ -2949,27 +2949,31 @@ g_get_current_dir (void)
       dotbuf.st_dev == pwdbuf.st_dev && dotbuf.st_ino == pwdbuf.st_ino)
     return g_strdup (pwd);
 
-  if (max_len == 0)
-    max_len = (G_PATH_LENGTH == -1) ? 2048 : G_PATH_LENGTH;
+  if (buffer_size == 0)
+    buffer_size = (G_PATH_LENGTH == -1) ? 2048 : G_PATH_LENGTH;
 
-  while (max_len < G_MAXULONG / 2)
+  while (buffer_size < G_MAXSIZE / 2)
     {
       g_free (buffer);
-      buffer = g_new (gchar, max_len + 1);
+      buffer = g_new (gchar, buffer_size);
       *buffer = 0;
-      dir = getcwd (buffer, max_len);
+      dir = getcwd (buffer, buffer_size);
 
       if (dir || errno != ERANGE)
         break;
 
-      max_len *= 2;
+      buffer_size *= 2;
     }
 
+  /* Check that getcwd() nul-terminated the string. It should do, but the specs
+   * don’t actually explicitly state that:
+   * https://pubs.opengroup.org/onlinepubs/9699919799/functions/getcwd.html */
+  g_assert (dir == NULL || strnlen (dir, buffer_size) < buffer_size);
+
   if (!dir || !*buffer)
     {
-      /* hm, should we g_error() out here?
-       * this can happen if e.g. "./" has mode \0000
-       */
+      /* Fallback return value */
+      g_assert (buffer_size >= 2);
       buffer[0] = G_DIR_SEPARATOR;
       buffer[1] = 0;
     }