124 lines
4.4 KiB
Diff
124 lines
4.4 KiB
Diff
From 3b12cfac29dddd27f1f166a7574d8374cc1dccf2 Mon Sep 17 00:00:00 2001
|
|
From: David Zeuthen <davidz@redhat.com>
|
|
Date: Fri, 01 Apr 2011 16:13:15 +0000
|
|
Subject: pkexec: Avoid TOCTTOU problems with parent process
|
|
|
|
In a nutshell, the parent process may change its uid (either real- or
|
|
effective uid) after launching pkexec. It can do this by exec()'ing
|
|
e.g. a setuid root program.
|
|
|
|
To avoid this problem, just use the uid the parent process had when it
|
|
executed pkexec. This happens to be the same uid of the pkexec process
|
|
itself.
|
|
|
|
Additionally, remove some dubious code that allowed pkexec to continue
|
|
when the parent process died as there is no reason to support
|
|
something like that. Also ensure that the pkexec process is killed if
|
|
the parent process dies.
|
|
|
|
This problem was pointed out by Neel Mehta <nmehta@google.com>.
|
|
|
|
Signed-off-by: David Zeuthen <davidz@redhat.com>
|
|
---
|
|
diff --git a/src/programs/pkexec.c b/src/programs/pkexec.c
|
|
index 9217954..3e656be 100644
|
|
--- a/src/programs/pkexec.c
|
|
+++ b/src/programs/pkexec.c
|
|
@@ -35,6 +35,10 @@
|
|
#include <pwd.h>
|
|
#include <errno.h>
|
|
|
|
+#ifdef __linux__
|
|
+#include <sys/prctl.h>
|
|
+#endif
|
|
+
|
|
#include <glib/gi18n.h>
|
|
|
|
#ifdef POLKIT_AUTHFW_PAM
|
|
@@ -423,7 +427,6 @@ main (int argc, char *argv[])
|
|
GPtrArray *saved_env;
|
|
gchar *opt_user;
|
|
pid_t pid_of_caller;
|
|
- uid_t uid_of_caller;
|
|
gpointer local_agent_handle;
|
|
|
|
ret = 127;
|
|
@@ -598,40 +601,49 @@ main (int argc, char *argv[])
|
|
*/
|
|
g_type_init ();
|
|
|
|
- /* now check if the program that invoked us is authorized */
|
|
+ /* make sure we are nuked if the parent process dies */
|
|
+#ifdef __linux__
|
|
+ if (prctl (PR_SET_PDEATHSIG, SIGTERM) != 0)
|
|
+ {
|
|
+ g_printerr ("prctl(PR_SET_PDEATHSIG, SIGTERM) failed: %s\n", g_strerror (errno));
|
|
+ goto out;
|
|
+ }
|
|
+#else
|
|
+#warning "Please add OS specific code to catch when the parent dies"
|
|
+#endif
|
|
+
|
|
+ /* Figure out the parent process */
|
|
pid_of_caller = getppid ();
|
|
if (pid_of_caller == 1)
|
|
{
|
|
/* getppid() can return 1 if the parent died (meaning that we are reaped
|
|
- * by /sbin/init); get process group leader instead - for example, this
|
|
- * happens when launching via gnome-panel (alt+f2, then 'pkexec gedit').
|
|
+ * by /sbin/init); In that case we simpy bail.
|
|
*/
|
|
- pid_of_caller = getpgrp ();
|
|
- }
|
|
-
|
|
- subject = polkit_unix_process_new (pid_of_caller);
|
|
- if (subject == NULL)
|
|
- {
|
|
- g_printerr ("No such process for pid %d: %s\n", (gint) pid_of_caller, error->message);
|
|
- g_error_free (error);
|
|
+ g_printerr ("Refusing to render service to dead parents.\n");
|
|
goto out;
|
|
}
|
|
|
|
- /* paranoia: check that the uid of pid_of_caller matches getuid() */
|
|
- error = NULL;
|
|
- uid_of_caller = polkit_unix_process_get_owner (POLKIT_UNIX_PROCESS (subject),
|
|
- &error);
|
|
- if (error != NULL)
|
|
- {
|
|
- g_printerr ("Error determing pid of caller (pid %d): %s\n", (gint) pid_of_caller, error->message);
|
|
- g_error_free (error);
|
|
- goto out;
|
|
- }
|
|
- if (uid_of_caller != getuid ())
|
|
- {
|
|
- g_printerr ("User of caller (%d) does not match our uid (%d)\n", uid_of_caller, getuid ());
|
|
- goto out;
|
|
- }
|
|
+ /* This process we want to check an authorization for is the process
|
|
+ * that launched us - our parent process.
|
|
+ *
|
|
+ * At the time the parent process fork()'ed and exec()'ed us, the
|
|
+ * process had the same real-uid that we have now. So we use this
|
|
+ * real-uid instead of of looking it up to avoid TOCTTOU issues
|
|
+ * (consider the parent process exec()'ing a setuid helper).
|
|
+ *
|
|
+ * On the other hand, the monotonic process start-time is guaranteed
|
|
+ * to never change so it's safe to look that up given only the PID
|
|
+ * since we are guaranteed to be nuked if the parent goes away
|
|
+ * (cf. the prctl(2) call above).
|
|
+ */
|
|
+ subject = polkit_unix_process_new_for_owner (pid_of_caller,
|
|
+ 0, /* 0 means "look up start-time in /proc" */
|
|
+ getuid ());
|
|
+ /* really double-check the invariants guaranteed by the PolkitUnixProcess class */
|
|
+ g_assert (subject != NULL);
|
|
+ g_assert (polkit_unix_process_get_pid (POLKIT_UNIX_PROCESS (subject)) == pid_of_caller);
|
|
+ g_assert (polkit_unix_process_get_uid (POLKIT_UNIX_PROCESS (subject)) >= 0);
|
|
+ g_assert (polkit_unix_process_get_start_time (POLKIT_UNIX_PROCESS (subject)) > 0);
|
|
|
|
error = NULL;
|
|
authority = polkit_authority_get_sync (NULL /* GCancellable* */, &error);
|
|
--
|
|
cgit v0.8.3-6-g21f6
|