RFC: Check permissions of .gdbinit files

[Daniel Jacobowitz <drow at false dot org>]

Gentoo recently published a security update for GDB, citing the fact that
GDB would load .gdbinit from the current directory even if that was owned by
another user.  I'm not sure how I feel about running GDB in an untrusted
directory or on untrusted binaries and expecting it to behave sensibly, but
this particular issue is easy to fix.  Here's my suggested fix; it's not the
same as Gentoo's.  If .gdbinit is world writable or owned by a different
user, refuse to open it (and warn the user).

Anyone have opinions on this change?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-05-30  Daniel Jacobowitz  <dan@codesourcery.com>

	* Makefile.in (cli-cmds.o): Update.
	* main.c (captured_main): Pass -1 to source_command when loading
	gdbinit files.
	* cli/cli-cmds.c: Include "gdb_stat.h" and <fcntl.h>.
	(source_command): Update documentation.  Check permissions if
	FROM_TTY is -1.

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.733
diff -u -p -r1.733 Makefile.in
--- Makefile.in	22 May 2005 20:36:18 -0000	1.733
+++ Makefile.in	30 May 2005 18:46:16 -0000
@@ -2766,7 +2766,7 @@ cli-cmds.o: $(srcdir)/cli/cli-cmds.c $(d
 	$(expression_h) $(frame_h) $(value_h) $(language_h) $(filenames_h) \
 	$(objfiles_h) $(source_h) $(disasm_h) $(ui_out_h) $(top_h) \
 	$(cli_decode_h) $(cli_script_h) $(cli_setshow_h) $(cli_cmds_h) \
-	$(tui_h)
+	$(tui_h) $(gdb_stat_h)
 	$(CC) -c $(INTERNAL_CFLAGS) $(srcdir)/cli/cli-cmds.c
 cli-decode.o: $(srcdir)/cli/cli-decode.c $(defs_h) $(symtab_h) \
 	$(gdb_regex_h) $(gdb_string_h) $(ui_out_h) $(cli_cmds_h) \
Index: main.c
===================================================================
RCS file: /cvs/src/src/gdb/main.c,v
retrieving revision 1.51
diff -u -p -r1.51 main.c
--- main.c	2 Apr 2005 20:25:22 -0000	1.51
+++ main.c	30 May 2005 18:46:16 -0000
@@ -604,7 +604,7 @@ extern int gdbtk_test (char *);
 
       if (!inhibit_gdbinit)
 	{
-	  catch_command_errors (source_command, homeinit, 0, RETURN_MASK_ALL);
+	  catch_command_errors (source_command, homeinit, -1, RETURN_MASK_ALL);
 	}
 
       /* Do stats; no need to do them elsewhere since we'll only
@@ -691,7 +691,7 @@ extern int gdbtk_test (char *);
       || memcmp ((char *) &homebuf, (char *) &cwdbuf, sizeof (struct stat)))
     if (!inhibit_gdbinit)
       {
-	catch_command_errors (source_command, gdbinit, 0, RETURN_MASK_ALL);
+	catch_command_errors (source_command, gdbinit, -1, RETURN_MASK_ALL);
       }
 
   for (i = 0; i < ncmd; i++)
Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.61
diff -u -p -r1.61 cli-cmds.c
--- cli/cli-cmds.c	27 May 2005 04:39:33 -0000	1.61
+++ cli/cli-cmds.c	30 May 2005 18:46:17 -0000
@@ -37,6 +37,7 @@
 #include "objfiles.h"
 #include "source.h"
 #include "disasm.h"
+#include "gdb_stat.h"
 
 #include "ui-out.h"
 
@@ -50,6 +51,8 @@
 #include "tui/tui.h"		/* For tui_active et.al.   */
 #endif
 
+#include <fcntl.h>
+
 /* Prototypes for local command functions */
 
 static void complete_command (char *, int);
@@ -419,30 +422,54 @@ cd_command (char *dir, int from_tty)
     pwd_command ((char *) 0, 1);
 }
 
+/* Load a GDB command file whose name is given in ARGS.  FROM_TTY may
+   be -1, in which case we are loading a gdbinit file; in that case,
+   be paranoid about unsafe files.  */
+
 void
 source_command (char *args, int from_tty)
 {
-  FILE *stream;
+  FILE *stream = NULL;
+  int fd;
   struct cleanup *old_cleanups;
   char *file = args;
 
   if (file == NULL)
-    {
-      error (_("source command requires pathname of file to source."));
-    }
+    error (_("source command requires pathname of file to source."));
 
   file = tilde_expand (file);
   old_cleanups = make_cleanup (xfree, file);
 
-  stream = fopen (file, FOPEN_RT);
-  if (!stream)
+  fd = open (file, O_RDONLY);
+  if (fd != -1)
+    stream = fdopen (fd, FOPEN_RT);
+  if (stream == NULL)
     {
-      if (from_tty)
+      if (from_tty > 0)
 	perror_with_name (file);
       else
 	return;
     }
 
+#ifdef HAVE_GETUID
+  if (from_tty == -1)
+    {
+      struct stat statbuf;
+      if (fstat (fd, &statbuf) < 0)
+	{
+	  perror_with_name (file);
+	  fclose (stream);
+	  return;
+	}
+      if (statbuf.st_uid != getuid () || (statbuf.st_mode & S_IWOTH))
+	{
+	  warning (_("not using untrusted file \"%s\""), file);
+	  fclose (stream);
+	  return;
+	}
+    }
+#endif
+
   script_from_file (stream, file);
 
   do_cleanups (old_cleanups);