This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: Check permissions of .gdbinit files


> 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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]