Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Keith Seitz <keiths@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFA] .gdbinit security (revived) [incl doc]
Date: Sat, 20 Nov 2010 09:45:00 -0000	[thread overview]
Message-ID: <83d3q0babs.fsf@gnu.org> (raw)
In-Reply-To: <4CE702E7.4050504@redhat.com>

> Date: Fri, 19 Nov 2010 15:06:15 -0800
> From: Keith Seitz <keiths@redhat.com>
> 
> A long time ago, Daniel posted a patch which would do a security check 
> of .gdbinit files and refuse to execute them if they were untrusted. See 
> http://sourceware.org/ml/gdb-patches/2005-05/msg00637.html . I would 
> like to resurrect that discussion.
> 
> At the time, there was some debate about whether simply refusing to read 
> the file was particularly user-unfriendly for a lot of developers. 
> Someone suggested adding an option to override the behavior and so on. 
> Overall, people agreed that doing something was correct.
> 
> I have implemented a slightly different option: ask the user if he would 
> like to run the untrusted file any way, much like removing a 
> write-protected file IMO.

Thanks.

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?

I'm also unsure whether we should disregard this issue on Windows.  If
it's important to make sure .gdbinit is safe, it should also be
important to do that on Windows (using the NTFS file security calls).

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.

> +	  warning (_("file \"%s\" is untrusted"), file);

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.

> +	  if (!query (_("Read file anyway? ")))

This could be automatically answered YES in some situations.  Do we
care?

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

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

The doco part is OK with those changes.


  parent reply	other threads:[~2010-11-20  9:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-19 23:10 Keith Seitz
2010-11-20  2:50 ` Jan Kratochvil
2010-11-23 17:15   ` Keith Seitz
2010-11-20  9:45 ` Eli Zaretskii [this message]
2010-11-23 18:31   ` Keith Seitz
2010-11-23 19:19     ` Eli Zaretskii
2010-11-23 23:19     ` Doug Evans
2010-11-30  0:23       ` Keith Seitz
2010-11-24 21:23     ` Jan Kratochvil
2010-11-24 21:27       ` Keith Seitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83d3q0babs.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=keiths@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox