This is the mail archive of the
libc-hacker@cygnus.com
mailing list for the glibc project.
Re: pt_chown
- To: Geoff Keating <geoffk@ozemail.com.au>
- Subject: Re: pt_chown
- From: Zack Weinberg <zack@rabi.columbia.edu>
- Date: Tue, 30 Mar 1999 21:06:56 -0500
- cc: libc-hacker@cygnus.com
On Wed, 31 Mar 1999 10:34:16 +1000, Geoff Keating wrote:
>> Date: Mon, 29 Mar 1999 10:30:10 -0500
>> From: Mark Kettenis <kettenis@gnu.org>
>>
>> PR 1046 reports a security problem in pt_chown (for which I'll send a
>> patch later today). The author also questions the use of argument
>> parsing and localisation in the program. I tend to disagree with him
>> since the argp and gettext code should be safe to use in a setuid
>> program (actually using it in pt_chown might help us catching bugs).
>> But since I wrote big parts of the code I may not be very impartial in
>> this matter. Does any of us have an opinion on the issue.
>
>su(1) uses gettext and getopt_long from glibc. Certainly, gettext
>must be safe in setuid programs. I don't see why argp should be
>considered to be less safe than getopt_long, and I'd expect it's
>already used in privileged code.
pt_chown also uses nsswitch, via getgrnam. I'd be a lot more worried
about that, nsswitch seems to me more likely to have bugs.
That said, here's a patch that makes pt_chown drop privileges before
calling argp or gettext; normal execution doesn't need either. It
also fixes the real security problem reported in the PR (race due to
closing the master pty).
- Do we have user side support for the capability stuff in 2.[12] yet?
zw
1999-03-30 20:59 -0500 Zack Weinberg <zack@rabi.phys.columbia.edu>
* login/programs/pt_chown.c: Drop privileges if invoked with
arguments. Don't close the master pty.
===================================================================
Index: login/programs/pt_chown.c
--- login/programs/pt_chown.c 1999/01/12 00:18:37 1.4
+++ login/programs/pt_chown.c 1999/03/31 01:54:11
@@ -55,7 +55,7 @@
static void
print_version (FILE *stream, struct argp_state *state)
{
- fprintf (stream, "pt_chmod (GNU %s) %s\n", PACKAGE, VERSION);
+ fprintf (stream, "pt_chown (GNU %s) %s\n", PACKAGE, VERSION);
fprintf (stream, gettext ("\
Copyright (C) %s Free Software Foundation, Inc.\n\
This is free software; see the source for copying conditions. There is NO\n\
@@ -95,41 +95,17 @@
}
int
-main (int argc, char *argv[])
+do_pt_chown (void)
{
char *pty;
- int remaining;
struct stat st;
struct group *p;
gid_t gid;
- /* Set locale via LC_ALL. */
- setlocale (LC_ALL, "");
-
- /* Set the text message domain. */
- textdomain (PACKAGE);
-
- /* parse and process arguments. */
- argp_parse (&argp, argc, argv, 0, &remaining, NULL);
-
- if (remaining < argc)
- {
- /* We should not be called with any non-option parameters. */
- error (0, 0, gettext ("too many arguments"));
- argp_help (&argp, stdout, ARGP_HELP_SEE | ARGP_HELP_EXIT_ERR,
- program_invocation_short_name);
- exit (EXIT_FAILURE);
- }
-
- /* Check if we are properly installed. */
- if (geteuid () != 0)
- error (FAIL_EXEC, 0, gettext ("needs to be installed setuid `root'"));
-
/* Check that PTY_FILENO is a valid master pseudo terminal. */
pty = ptsname (PTY_FILENO);
if (pty == NULL)
return errno == EBADF ? FAIL_EBADF : FAIL_EINVAL;
- close (PTY_FILENO);
/* Check that the returned slave pseudo terminal is a
character device. */
@@ -149,6 +125,46 @@
and writable by the group. */
if (chmod (pty, S_IRUSR|S_IWUSR|S_IWGRP) < 0)
return FAIL_EACCES;
+
+ return 0;
+}
+
+
+int
+main (int argc, char *argv[])
+{
+ int remaining;
+
+ /* Normal invocation of this program is with no arguments and
+ with privileges.
+ FIXME: Should use capable (CAP_CHOWN|CAP_FOWNER). */
+ if (argc == 1 && geteuid () == 0)
+ return do_pt_chown ();
+
+ /* We aren't going to be using privileges, so drop them right now. */
+ setuid (getuid ());
+
+ /* Set locale via LC_ALL. */
+ setlocale (LC_ALL, "");
+
+ /* Set the text message domain. */
+ textdomain (PACKAGE);
+
+ /* parse and process arguments. */
+ argp_parse (&argp, argc, argv, 0, &remaining, NULL);
+
+ if (remaining < argc)
+ {
+ /* We should not be called with any non-option parameters. */
+ error (0, 0, gettext ("too many arguments"));
+ argp_help (&argp, stdout, ARGP_HELP_SEE | ARGP_HELP_EXIT_ERR,
+ program_invocation_short_name);
+ return EXIT_FAILURE;
+ }
+
+ /* Check if we are properly installed. */
+ if (geteuid () != 0)
+ error (FAIL_EXEC, 0, gettext ("needs to be installed setuid `root'"));
- exit (EXIT_SUCCESS);
+ return EXIT_SUCCESS;
}