From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18339 invoked by alias); 30 May 2005 22:42:07 -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 18329 invoked by uid 22791); 30 May 2005 22:42:02 -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 22:42:02 +0000 Received: from drow by nevyn.them.org with local (Exim 4.50) id 1DcsxM-0000nJ-Ot; Mon, 30 May 2005 18:42:00 -0400 Date: Mon, 30 May 2005 23:00:00 -0000 From: Daniel Jacobowitz To: Eli Zaretskii Cc: gdb-patches@sourceware.org Subject: Re: RFC: Check permissions of .gdbinit files Message-ID: <20050530224200.GB2727@nevyn.them.org> Mail-Followup-To: Eli Zaretskii , gdb-patches@sourceware.org References: <20050530185201.GA29332@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.8i X-SW-Source: 2005-05/txt/msg00654.txt.bz2 On Tue, May 31, 2005 at 12:54:53AM +0300, Eli Zaretskii wrote: > > Date: Mon, 30 May 2005 14:52:01 -0400 > > From: Daniel Jacobowitz > > > > 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. Bother; I thought about the portability for a while, but didn't quite consider this. We're still OK though - the whole thing is surrounded by HAVE_GETUID, and MinGW does not have GETUID, if I understand correctly. I think it's plausible to assume that S_IWOTH will be defined if getuid() is; does that seem reasonable to you? > > + 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). I was pretty sure that the convention was lowercase letter, and no trailing period. But I don't think this is documented anywhere, and the source is wildly inconsistent. I could easily be remembering wrong :-) > > - 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'? In order to have the file descriptor, for fstat. It occurs to me now that "fileno" could be used for this; I am not sure how portable fileno is (my system's man page for it is somewhat ambiguous on the subject) but I see that we use it already, so the answer is "portable enough". That would probably be cleaner. > > + if (statbuf.st_uid != getuid () || (statbuf.st_mode & S_IWOTH)) > > Shouldn't we allow this to go unnoticed for root? No, that's especially when we shouldn't. It's root that wants to be paranoid about silently using some user's .gdbinit file. This is the opposite problem to access checks in a privileged program; we want no more "privileges" for the superuser here. > > + 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. Where would you suggest? Ah, never mind, I see there is a section on .gdbinit already. -- Daniel Jacobowitz CodeSourcery, LLC