This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: RFC: Check permissions of .gdbinit files
- From: Eli Zaretskii <eliz at gnu dot org>
- To: gdb-patches at sourceware dot org
- Date: Tue, 31 May 2005 00:54:53 +0300
- Subject: Re: RFC: Check permissions of .gdbinit files
- References: <20050530185201.GA29332@nevyn.them.org>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> Date: Mon, 30 May 2005 14:52:01 -0400
> From: Daniel Jacobowitz <drow@false.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?
Hmm... bother. This change might break some of the non-Posix
platforms. Perhaps I missed something, but AFAICS MinGW lacks the
definition of S_IWOTH, so this will fail to compile for MinGW. But
even if it did compile, the MinGW version of `fstat' returns the
S_IWOTH bit set for all files, so the MinGW port will probably always
display the warning.
The DJGPP port will not be affected: it does have S_IWOTH and it
reports the S_IWOTH bit as reset. Can't easily check Cygwin here, but
I'm guessing it will do TRT here as well.
> + error (_("source command requires pathname of file to source."));
I think the message text should begin with a capital letter (yes, I
know the original didn't do it, either).
> - stream = fopen (file, FOPEN_RT);
> - if (!stream)
> + fd = open (file, O_RDONLY);
> + if (fd != -1)
> + stream = fdopen (fd, FOPEN_RT);
Could you please tell why you replaced `fopen' with `open'+`fdopen'?
> + if (statbuf.st_uid != getuid () || (statbuf.st_mode & S_IWOTH))
Shouldn't we allow this to go unnoticed for root?
> + warning (_("not using untrusted file \"%s\""), file);
I think the message text should begin with a capital letter.
Last, but not least: if we decide to make such a change (which to my
HO sounds like a good idea, in general), we should describe this
subtlety (and the warning it could produce) in the user's manual.