Hi, Eli,

On 11/20/2010 01:42 AM, Eli Zaretskii wrote:
> In that discussion, Andreas suggested to avoid the warning if the user
> belongs to the same group as the file's owner.  I don't see your patch
> addressing that part.  Why not?

It was not clear to me that this was generally considered desirable. 
Daniel noted, "I'm trying not to encode too much site policy
into GDB".

I can add this, but to be honest, I'm no longer certain how to write a 
test for this, since it is not possible (AFAIK) to change permissions of 
a file in the test suite that would cause this to trigger.

> I realize that it would be inappropriate to ask you to do that as a
> prerequisite for accepting the patch, but maybe a TODO comment should
> be placed there about the Windows case.  Then someone else could do
> that at some point.

Done.

> I would suggest to spell out why it is untrusted.  Otherwise the
> warning sounds grave, but doesn't give enough information to make the
> decision.

Done.

>
>> +	  if (!query (_("Read file anyway? ")))
>
> This could be automatically answered YES in some situations.  Do we
> care?

Good question. I always seem to forget that the user can do this. To err 
on the side of safety, I've changed to using nquery.

>> +If @file{.gdbinit} is untrusted (it is not owned by the current user
>> +or the file is world-writable), @value{GDBN} will warn the user and ask
>
> This should be qualified by "on some platforms", because not every
> platform that supports file ownership will issue this warning.

Yes, indeed. Fixed.

> And a minor stylistic issue.  You say "it is not owned" and then "the
> file is world-writable".  This is inconsistent, and could confuse the
> reader into thinking that "it" and "the file" are two different
> things.  Suggest to rephrase:
>
>    If @file{.gdbinit} is @dfn{untrusted} (either not owned by the
>    current user or world-writable), ...

You are absolutely correct. I've added a bit about the group ID addition 
and reworded this to help readability (I hope):

"On some platorms, @value{GDBN} will perform security check of
@file{.gdbinit} before it is executed.  If @file{.gdbinit} is not owned
by the current user or the file is world-writable, @value{GDBN} will
warn the user and ask if the file should be read anyway.  These warnings 
are suppressed when the group ID of the file's owner matches the group 
ID of the user."

I've attached an updated version of the patch (without the test case, 
which I don't think can work without some sort of administrative 
permissions).

Keith

ChangeLog
2010-11-23  Keith Seitz  <keiths@redhat.com>

	From  Daniel Jacobowitz  <dan@codesourcery.com>
	and Jeff Johnston  <jjohnstn@redhat.com>:
	* cli/cli-cmds.h (find_and_open_script): Add from_tty argument.
	* cli/cli-cmds.c (find_and_open_script): Likewise.  When
	from_tty is -1, perform a security check of the file.  If it
	fails, warn the user and whether he wants to read the file anyway.
	(source_script_with_search): Update call to find_and_open_script.
	Only print an error if from_tty is greater than zero.
	* main.c (captured_main): Pass from_tty = -1 when sourcing
	gdbinit files.
	* python/py-auto-load.c (source_section_scripts): Update call
	to find_and_open_script.

doc/ChangeLog
2010-11-23  Keith Seitz  <keiths@redhat.com>

	* gdb.texinfo (Startup): Document security handling of
	.gdbinit files.