From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7246 invoked by alias); 30 May 2005 18:52:11 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 6919 invoked by uid 22791); 30 May 2005 18:52:03 -0000 Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Mon, 30 May 2005 18:52:03 +0000 Received: from drow by nevyn.them.org with local (Exim 4.50) id 1DcpMn-0007e0-7q for gdb-patches@sourceware.org; Mon, 30 May 2005 14:52:01 -0400 Date: Mon, 30 May 2005 19:41:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sourceware.org Subject: RFC: Check permissions of .gdbinit files Message-ID: <20050530185201.GA29332@nevyn.them.org> Mail-Followup-To: gdb-patches@sourceware.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.8i X-SW-Source: 2005-05/txt/msg00639.txt.bz2 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 * 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 . (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 + /* 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);