From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32698 invoked by alias); 23 Nov 2010 23:19:17 -0000 Received: (qmail 32687 invoked by uid 22791); 23 Nov 2010 23:19:16 -0000 X-SWARE-Spam-Status: No, hits=-4.7 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.35) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 23 Nov 2010 23:19:01 +0000 Received: from wpaz1.hot.corp.google.com (wpaz1.hot.corp.google.com [172.24.198.65]) by smtp-out.google.com with ESMTP id oANNIE8p015988 for ; Tue, 23 Nov 2010 15:18:14 -0800 Received: from qyk12 (qyk12.prod.google.com [10.241.83.140]) by wpaz1.hot.corp.google.com with ESMTP id oANNHRbd009430 for ; Tue, 23 Nov 2010 15:18:12 -0800 Received: by qyk12 with SMTP id 12so2359231qyk.5 for ; Tue, 23 Nov 2010 15:18:12 -0800 (PST) MIME-Version: 1.0 Received: by 10.229.249.203 with SMTP id ml11mr7207454qcb.266.1290554291416; Tue, 23 Nov 2010 15:18:11 -0800 (PST) Received: by 10.220.185.203 with HTTP; Tue, 23 Nov 2010 15:18:11 -0800 (PST) In-Reply-To: <4CEC0757.6000503@redhat.com> References: <4CE702E7.4050504@redhat.com> <83d3q0babs.fsf@gnu.org> <4CEC0757.6000503@redhat.com> Date: Tue, 23 Nov 2010 23:19:00 -0000 Message-ID: Subject: Re: [RFA] .gdbinit security (revived) [incl doc] From: Doug Evans To: Keith Seitz Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true 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/msg00381.txt.bz2 On Tue, Nov 23, 2010 at 10:26 AM, Keith Seitz wrote: > [...] > ChangeLog > 2010-11-23 =A0Keith Seitz =A0 > > =A0 =A0 =A0 =A0From =A0Daniel Jacobowitz =A0 > =A0 =A0 =A0 =A0and Jeff Johnston =A0: > =A0 =A0 =A0 =A0* cli/cli-cmds.h (find_and_open_script): Add from_tty argu= ment. > =A0 =A0 =A0 =A0* cli/cli-cmds.c (find_and_open_script): Likewise. =A0When > =A0 =A0 =A0 =A0from_tty is -1, perform a security check of the file. =A0I= f it > =A0 =A0 =A0 =A0fails, warn the user and whether he wants to read the file= anyway. > =A0 =A0 =A0 =A0(source_script_with_search): Update call to find_and_open_= script. > =A0 =A0 =A0 =A0Only print an error if from_tty is greater than zero. > =A0 =A0 =A0 =A0* main.c (captured_main): Pass from_tty =3D -1 when sourci= ng > =A0 =A0 =A0 =A0gdbinit files. > =A0 =A0 =A0 =A0* python/py-auto-load.c (source_section_scripts): Update c= all > =A0 =A0 =A0 =A0to find_and_open_script. > > doc/ChangeLog > 2010-11-23 =A0Keith Seitz =A0 > > =A0 =A0 =A0 =A0* gdb.texinfo (Startup): Document security handling of > =A0 =A0 =A0 =A0.gdbinit files. Hi. A few comments inline. >- catch_command_errors (source_script, home_gdbinit, 0, RETURN_MASK_ALL= ); >+ catch_command_errors (source_script, home_gdbinit, -1, RETURN_MASK_AL= L); I don't mind using -1 for from_tty here (especially if there is precedent :-)), but a #define/enum would be nicer. catch_command_errors has a limited API so overloading from_tty is a pragmatic tradeoff. Feel free to save for a separate patch. Just mentioning it to prime the pumps doing something like this down the road. >+ If FROM_TTY is -1, then this script is being automatically loaded >+ at runtime, and a security check will be performed on the file >+ (supported only on hosts with HAVE_GETUID). We're combining two concepts here: "is the command from the tty?" and "do security checks?". IWBN to keep them separate here. Maybe specify both separately or just have check_security instead of from_t= ty? >+ if (statbuf.st_uid !=3D getuid ()) I wonder if you also need to watch for file owner =3D=3D root (and not world writable). E.g. scripts like --with-system-gdbinit. That won't happen with the patch as is, but that feels like a high-level detail that this function shouldn't have to know about. Then again, why not do this security check for system.gdbinit too? > opened =3D find_and_open_script (file, 1 /*search_path*/, >- &stream, &full_path); >+ &stream, &full_path, 1 /* from_tty */); Passing 1 for from_tty feels wrong here. If find_and_open_script had a check_security parameter instead of from_tty, then one could just pass 0 here.