[RFA] .gdbinit security (revived) [incl doc]
Doug Evans
dje@google.com
Tue Nov 23 23:19:00 GMT 2010
On Tue, Nov 23, 2010 at 10:26 AM, Keith Seitz <keiths@redhat.com> wrote:
> [...]
> 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.
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_ALL);
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_tty?
>+ if (statbuf.st_uid != getuid ())
I wonder if you also need to watch for file owner == 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 = 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.
More information about the Gdb-patches
mailing list