[glib: 6/8] gspawn: Clarify that empty argv arrays are not allowed when spawning




commit 44f4d551503005f1dad2ed91530b9bec1a7b47ed
Author: Philip Withnall <pwithnall endlessos org>
Date:   Mon Jan 31 14:54:10 2022 +0000

    gspawn: Clarify that empty argv arrays are not allowed when spawning
    
    While `execve()` might allow (probably malicious) users to execute a
    program with an empty `argv` array, gspawn does not. It’s not actually
    possible, as the path to the binary to execute is not specified
    separately from the argument array.
    
    Explicitly document and encode that in preconditions.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>

 glib/gspawn-win32.c | 15 ++++++++++-----
 glib/gspawn.c       | 24 ++++++++++++++++++------
 2 files changed, 28 insertions(+), 11 deletions(-)
---
diff --git a/glib/gspawn-win32.c b/glib/gspawn-win32.c
index 02eddccf1..254195c08 100644
--- a/glib/gspawn-win32.c
+++ b/glib/gspawn-win32.c
@@ -236,7 +236,7 @@ g_spawn_async (const gchar          *working_directory,
                GPid                 *child_pid,
                GError              **error)
 {
-  g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (argv != NULL && argv[0] != NULL, FALSE);
   
   return g_spawn_async_with_pipes (working_directory,
                                    argv, envp,
@@ -460,6 +460,8 @@ do_spawn_directly (gint                 *exit_status,
   gint conv_error_index;
   wchar_t *wargv0, **wargv, **wenvp;
 
+  g_assert (argv != NULL && argv[0] != NULL);
+
   new_argv = (flags & G_SPAWN_FILE_AND_ARGV_ZERO) ? protected_argv + 1 : protected_argv;
       
   wargv0 = g_utf8_to_utf16 (argv[0], -1, NULL, NULL, &conv_error);
@@ -601,6 +603,7 @@ fork_exec (gint                  *exit_status,
   int stdout_pipe[2] = { -1, -1 };
   int stderr_pipe[2] = { -1, -1 };
 
+  g_assert (argv != NULL && argv[0] != NULL);
   g_assert (stdin_pipe_out == NULL || stdin_fd < 0);
   g_assert (stdout_pipe_out == NULL || stdout_fd < 0);
   g_assert (stderr_pipe_out == NULL || stderr_fd < 0);
@@ -989,7 +992,7 @@ g_spawn_sync (const gchar          *working_directory,
   gboolean failed;
   gint status;
   
-  g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (argv != NULL && argv[0] != NULL, FALSE);
   g_return_val_if_fail (!(flags & G_SPAWN_DO_NOT_REAP_CHILD), FALSE);
   g_return_val_if_fail (standard_output == NULL ||
                         !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
@@ -1221,7 +1224,7 @@ g_spawn_async_with_pipes (const gchar          *working_directory,
                           gint                 *standard_error,
                           GError              **error)
 {
-  g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (argv != NULL && argv[0] != NULL, FALSE);
   g_return_val_if_fail (standard_output == NULL ||
                         !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
   g_return_val_if_fail (standard_error == NULL ||
@@ -1263,7 +1266,7 @@ g_spawn_async_with_fds (const gchar          *working_directory,
                         gint                  stderr_fd,
                         GError              **error)
 {
-  g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (argv != NULL && argv[0] != NULL, FALSE);
   g_return_val_if_fail (stdin_fd == -1 ||
                         !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
   g_return_val_if_fail (stderr_fd == -1 ||
@@ -1312,7 +1315,7 @@ g_spawn_async_with_pipes_and_fds (const gchar           *working_directory,
                                   gint                  *stderr_pipe_out,
                                   GError               **error)
 {
-  g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (argv != NULL && argv[0] != NULL, FALSE);
   g_return_val_if_fail (stdout_pipe_out == NULL ||
                         !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
   g_return_val_if_fail (stderr_pipe_out == NULL ||
@@ -1359,6 +1362,7 @@ g_spawn_command_line_sync (const gchar  *command_line,
 
   g_return_val_if_fail (command_line != NULL, FALSE);
   
+  /* This will return a runtime error if @command_line is the empty string. */
   if (!g_shell_parse_argv (command_line,
                            NULL, &argv,
                            error))
@@ -1388,6 +1392,7 @@ g_spawn_command_line_async (const gchar *command_line,
 
   g_return_val_if_fail (command_line != NULL, FALSE);
 
+  /* This will return a runtime error if @command_line is the empty string. */
   if (!g_shell_parse_argv (command_line,
                            NULL, &argv,
                            error))
diff --git a/glib/gspawn.c b/glib/gspawn.c
index 1128221cc..cce03f81d 100644
--- a/glib/gspawn.c
+++ b/glib/gspawn.c
@@ -319,7 +319,7 @@ read_data (GString *str,
  * @working_directory: (type filename) (nullable): child's current working
  *     directory, or %NULL to inherit parent's
  * @argv: (array zero-terminated=1) (element-type filename):
- *     child's argument vector
+ *     child's argument vector, which must be non-empty and %NULL-terminated
  * @envp: (array zero-terminated=1) (element-type filename) (nullable):
  *     child's environment, or %NULL to inherit parent's
  * @flags: flags from #GSpawnFlags
@@ -378,6 +378,7 @@ g_spawn_sync (const gchar          *working_directory,
   gint status;
   
   g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (argv[0] != NULL, FALSE);
   g_return_val_if_fail (!(flags & G_SPAWN_DO_NOT_REAP_CHILD), FALSE);
   g_return_val_if_fail (standard_output == NULL ||
                         !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
@@ -578,7 +579,7 @@ g_spawn_sync (const gchar          *working_directory,
  * @working_directory: (type filename) (nullable): child's current working
  *     directory, or %NULL to inherit parent's, in the GLib file name encoding
  * @argv: (array zero-terminated=1) (element-type filename): child's argument
- *     vector, in the GLib file name encoding
+ *     vector, in the GLib file name encoding; it must be non-empty and %NULL-terminated
  * @envp: (array zero-terminated=1) (element-type filename) (nullable):
  *     child's environment, or %NULL to inherit parent's, in the GLib file
  *     name encoding
@@ -610,6 +611,7 @@ g_spawn_async_with_pipes (const gchar          *working_directory,
                           GError              **error)
 {
   g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (argv[0] != NULL, FALSE);
   g_return_val_if_fail (standard_output == NULL ||
                         !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
   g_return_val_if_fail (standard_error == NULL ||
@@ -646,7 +648,7 @@ g_spawn_async_with_pipes (const gchar          *working_directory,
  * @working_directory: (type filename) (nullable): child's current working
  *     directory, or %NULL to inherit parent's, in the GLib file name encoding
  * @argv: (array zero-terminated=1) (element-type filename): child's argument
- *     vector, in the GLib file name encoding
+ *     vector, in the GLib file name encoding; it must be non-empty and %NULL-terminated
  * @envp: (array zero-terminated=1) (element-type filename) (nullable):
  *     child's environment, or %NULL to inherit parent's, in the GLib file
  *     name encoding
@@ -880,6 +882,7 @@ g_spawn_async_with_pipes_and_fds (const gchar           *working_directory,
                                   GError               **error)
 {
   g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (argv[0] != NULL, FALSE);
   g_return_val_if_fail (stdout_pipe_out == NULL ||
                         !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
   g_return_val_if_fail (stderr_pipe_out == NULL ||
@@ -922,7 +925,8 @@ g_spawn_async_with_pipes_and_fds (const gchar           *working_directory,
 /**
  * g_spawn_async_with_fds:
  * @working_directory: (type filename) (nullable): child's current working directory, or %NULL to inherit 
parent's, in the GLib file name encoding
- * @argv: (array zero-terminated=1): child's argument vector, in the GLib file name encoding
+ * @argv: (array zero-terminated=1): child's argument vector, in the GLib file name encoding;
+ *   it must be non-empty and %NULL-terminated
  * @envp: (array zero-terminated=1) (nullable): child's environment, or %NULL to inherit parent's, in the 
GLib file name encoding
  * @flags: flags from #GSpawnFlags
  * @child_setup: (scope async) (nullable): function to run in the child just before exec()
@@ -956,6 +960,7 @@ g_spawn_async_with_fds (const gchar          *working_directory,
                         GError              **error)
 {
   g_return_val_if_fail (argv != NULL, FALSE);
+  g_return_val_if_fail (argv[0] != NULL, FALSE);
   g_return_val_if_fail (stdout_fd < 0 ||
                         !(flags & G_SPAWN_STDOUT_TO_DEV_NULL), FALSE);
   g_return_val_if_fail (stderr_fd < 0 ||
@@ -1039,6 +1044,7 @@ g_spawn_command_line_sync (const gchar  *command_line,
 
   g_return_val_if_fail (command_line != NULL, FALSE);
   
+  /* This will return a runtime error if @command_line is the empty string. */
   if (!g_shell_parse_argv (command_line,
                            NULL, &argv,
                            error))
@@ -1086,6 +1092,7 @@ g_spawn_command_line_async (const gchar *command_line,
 
   g_return_val_if_fail (command_line != NULL, FALSE);
 
+  /* This will return a runtime error if @command_line is the empty string. */
   if (!g_shell_parse_argv (command_line,
                            NULL, &argv,
                            error))
@@ -1636,7 +1643,9 @@ enum
 };
 
 /* This function is called between fork() and exec() and hence must be
- * async-signal-safe (see signal-safety(7)) until it calls exec(). */
+ * async-signal-safe (see signal-safety(7)) until it calls exec().
+ *
+ * All callers must guarantee that @argv and @argv[0] are non-NULL. */
 static void
 do_exec (gint                  child_err_report_fd,
          gint                  stdin_fd,
@@ -1926,6 +1935,8 @@ do_posix_spawn (const gchar * const *argv,
   gsize i;
   int r;
 
+  g_assert (argv != NULL && argv[0] != NULL);
+
   if (*argv[0] == '\0')
     {
       /* We check the simple case first. */
@@ -2176,6 +2187,7 @@ fork_exec (gboolean              intermediate_child,
   gint n_child_close_fds = 0;
   gint *source_fds_copy = NULL;
 
+  g_assert (argv != NULL && argv[0] != NULL);
   g_assert (stdin_pipe_out == NULL || stdin_fd < 0);
   g_assert (stdout_pipe_out == NULL || stdout_fd < 0);
   g_assert (stderr_pipe_out == NULL || stderr_fd < 0);
@@ -2734,7 +2746,7 @@ g_execute (const gchar  *file,
            gchar        *search_path_buffer,
            gsize         search_path_buffer_len)
 {
-  if (*file == '\0')
+  if (file == NULL || *file == '\0')
     {
       /* We check the simple case first. */
       errno = ENOENT;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]