This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] .gdbinit security (revived) [incl doc]


Hi, Doug,

On 11/23/2010 03:18 PM, Doug Evans wrote:
-    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.
[snip]
Maybe specify both separately or just have check_security instead of
from_tty?

Actually, I've changed this a little bit on reflection. I've added a new wrapper which will call find_and_open_script with parameter "security_check" set. So I'm not overloading the use of from_tty at all; it can stay a simple "boolean."



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

Yeah, this is the problem: we're starting to get into a lot of detail here, and, like Daniel, I didn't really want to dig myself a hole on security issues. I'm far from an expert on that. Heck, I'm probably not too far from novice!


IMO, it is a delicate balancing act between adding a bunch of site-specific knowledge [What if the system-wide gdbinit was not installed by root, but by some other user/group?] and maintenance [Should we add configure options for which group/user to implicitly trust?]. It seems like it could all easily get out of control.

If a policy can be decided on, I am, of course, happy to follow it through.

Here are my thoughts on it. The goal is to prevent a gdbinit (or ANY script that may be automatically read) from automatically executing if it could possibly contain malicious commands.

"If it could possibly contain malicious commands" means (to me), "Can someone other than the user write to the file?" If so, gdb should warn and query the user. [I don't pretend to prevent stupid users from doing stupid things. It's a debugger; I like to think that all our users are intelligent people.]

Trying to formalize, I think this is:
1) If the script is world-writable --> warn/query the user
2) If the script is group-writable --> warn/query
3) If the script is not owned by you or root --> warn/query

Now I can accept an argument that #2 should be dropped, but for the sake of discussion, I've kept it in the attached patch.

Then again, why not do this security check for system.gdbinit too?

My only guess is that this is presumed "safe," since I suppose a trusted source installed/created that. But that is my only guess. I've included this check FWIW.


Full disclosure: There are two (or perhaps more?) other places where a security check should be performed that I haven't attempted to implement:
1) when sourcing a file from .gdbinit
2) in python autoloading?


Those are a little trickier and something not addressed by any of the patches out in the wild.

Comments?
Keith

ChangeLog
2010-11-29  Keith Seitz  <keiths@redhat.com>

	Based on work from  Daniel Jacobowitz  <dan@codesourcery.com>
	and Jeff Johnston  <jjohnstn@redhat.com>:
	* cli/cli-cmds.h (source_script_with_security_check): New
	function.
	* cli/cli-cmds.c (source_script_with_security_check): Likewise.
	(find_and_open_script): Add SECURITY_CHECK parameter.
	Implement a basic security check of the script file before
	executing it.
	(source_script_with_search): Add SECURITY_CHECK parameter and
	pass it to find_and_open_script.
	(source_script): Update call to find_and_open_script, performing
	no security check of the file.
	(source_command): Likewise.
	(source_script_with_security_check): New function.
	* main.c (captured_main): When reading init files, use
	source_script_with_security_check.
	* python/py-auto-load.c (source_section_scripts): Update call
	to find_and_open_script, performing no security check.

doc/ChangeLog
2010-11-29  Keith Seitz  <keiths@redhat.com>

	* gdb.texinfo (Startup): Document security handling of
	.gdbinit files.

Attachment: gdbinit-security-3.patch
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]