From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11092 invoked by alias); 20 Nov 2010 09:45:07 -0000 Received: (qmail 11081 invoked by uid 22791); 20 Nov 2010 09:45:06 -0000 X-SWARE-Spam-Status: No, hits=-0.3 required=5.0 tests=AWL,BAYES_00,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout20.012.net.il (HELO mtaout20.012.net.il) (80.179.55.166) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 20 Nov 2010 09:45:01 +0000 Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0LC600400GBJ3R00@a-mtaout20.012.net.il> for gdb-patches@sourceware.org; Sat, 20 Nov 2010 11:44:13 +0200 (IST) Received: from HOME-C4E4A596F7 ([77.124.52.47]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0LC6003CQGDONC70@a-mtaout20.012.net.il>; Sat, 20 Nov 2010 11:44:13 +0200 (IST) Date: Sat, 20 Nov 2010 09:45:00 -0000 From: Eli Zaretskii Subject: Re: [RFA] .gdbinit security (revived) [incl doc] In-reply-to: <4CE702E7.4050504@redhat.com> To: Keith Seitz Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83d3q0babs.fsf@gnu.org> References: <4CE702E7.4050504@redhat.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-11/txt/msg00290.txt.bz2 > Date: Fri, 19 Nov 2010 15:06:15 -0800 > From: Keith Seitz > > 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.