From 7d041de1f5d54d02b2e2115956fc28f439360eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 11 Jul 2018 18:48:21 +0200 Subject: [PATCH] Fix CVE-2018-1116 --- polkit-0.113-CVE-2018-1116.patch | 508 +++++++++++++++++++++++++++++++ polkit.spec | 8 +- 2 files changed, 515 insertions(+), 1 deletion(-) create mode 100644 polkit-0.113-CVE-2018-1116.patch diff --git a/polkit-0.113-CVE-2018-1116.patch b/polkit-0.113-CVE-2018-1116.patch new file mode 100644 index 0000000..6720c34 --- /dev/null +++ b/polkit-0.113-CVE-2018-1116.patch @@ -0,0 +1,508 @@ +diff -ur polkit/src/polkit/polkitprivate.h polkit-0.113/src/polkit/polkitprivate.h +--- polkit/src/polkit/polkitprivate.h 2014-01-14 23:42:25.000000000 +0100 ++++ polkit-0.113/src/polkit/polkitprivate.h 2018-07-11 18:25:38.812870344 +0200 +@@ -44,6 +44,8 @@ + GVariant *polkit_subject_to_gvariant (PolkitSubject *subject); + GVariant *polkit_identity_to_gvariant (PolkitIdentity *identity); + ++gint polkit_unix_process_get_racy_uid__ (PolkitUnixProcess *process, GError **error); ++ + PolkitSubject *polkit_subject_new_for_gvariant (GVariant *variant, GError **error); + PolkitIdentity *polkit_identity_new_for_gvariant (GVariant *variant, GError **error); + +diff -ur polkit/src/polkit/polkitunixprocess.c polkit-0.113/src/polkit/polkitunixprocess.c +--- polkit/src/polkit/polkitunixprocess.c 2014-01-14 23:42:25.000000000 +0100 ++++ polkit-0.113/src/polkit/polkitunixprocess.c 2018-07-11 18:26:49.561297405 +0200 +@@ -49,6 +49,14 @@ + * To uniquely identify processes, both the process id and the start + * time of the process (a monotonic increasing value representing the + * time since the kernel was started) is used. ++ * ++ * NOTE: This object stores, and provides access to, the real UID of the ++ * process. That value can change over time (with set*uid*(2) and exec*(2)). ++ * Checks whether an operation is allowed need to take care to use the UID ++ * value as of the time when the operation was made (or, following the open() ++ * privilege check model, when the connection making the operation possible ++ * was initiated). That is usually done by initializing this with ++ * polkit_unix_process_new_for_owner() with trusted data. + */ + + /** +@@ -83,9 +91,6 @@ + static guint64 get_start_time_for_pid (gint pid, + GError **error); + +-static gint _polkit_unix_process_get_owner (PolkitUnixProcess *process, +- GError **error); +- + #ifdef HAVE_FREEBSD + static gboolean get_kinfo_proc (gint pid, struct kinfo_proc *p); + #endif +@@ -170,7 +175,7 @@ + { + GError *error; + error = NULL; +- process->uid = _polkit_unix_process_get_owner (process, &error); ++ process->uid = polkit_unix_process_get_racy_uid__ (process, &error); + if (error != NULL) + { + process->uid = -1; +@@ -259,6 +264,12 @@ + * Gets the user id for @process. Note that this is the real user-id, + * not the effective user-id. + * ++ * NOTE: The UID may change over time, so the returned value may not match the ++ * current state of the underlying process; or the UID may have been set by ++ * polkit_unix_process_new_for_owner() or polkit_unix_process_set_uid(), ++ * in which case it may not correspond to the actual UID of the referenced ++ * process at all (at any point in time). ++ * + * Returns: The user id for @process or -1 if unknown. + */ + gint +@@ -655,18 +666,26 @@ + return start_time; + } + +-static gint +-_polkit_unix_process_get_owner (PolkitUnixProcess *process, +- GError **error) ++/* ++ * Private: Return the "current" UID. Note that this is inherently racy, ++ * and the value may already be obsolete by the time this function returns; ++ * this function only guarantees that the UID was valid at some point during ++ * its execution. ++ */ ++gint ++polkit_unix_process_get_racy_uid__ (PolkitUnixProcess *process, ++ GError **error) + { + gint result; + gchar *contents; + gchar **lines; ++ guint64 start_time; + #ifdef HAVE_FREEBSD + struct kinfo_proc p; + #else + gchar filename[64]; + guint n; ++ GError *local_error; + #endif + + g_return_val_if_fail (POLKIT_IS_UNIX_PROCESS (process), 0); +@@ -689,6 +708,7 @@ + } + + result = p.ki_uid; ++ start_time = (guint64) p.ki_start.tv_sec; + #else + + /* see 'man proc' for layout of the status file +@@ -722,17 +742,37 @@ + else + { + result = real_uid; +- goto out; ++ goto found; + } + } +- + g_set_error (error, + POLKIT_ERROR, + POLKIT_ERROR_FAILED, + "Didn't find any line starting with `Uid:' in file %s", + filename); ++ goto out; ++ ++found: ++ /* The UID and start time are, sadly, not available in a single file. So, ++ * read the UID first, and then the start time; if the start time is the same ++ * before and after reading the UID, it couldn't have changed. ++ */ ++ local_error = NULL; ++ start_time = get_start_time_for_pid (process->pid, &local_error); ++ if (local_error != NULL) ++ { ++ g_propagate_error (error, local_error); ++ goto out; ++ } + #endif + ++ if (process->start_time != start_time) ++ { ++ g_set_error (error, POLKIT_ERROR, POLKIT_ERROR_FAILED, ++ "process with PID %d has been replaced", process->pid); ++ goto out; ++ } ++ + out: + g_strfreev (lines); + g_free (contents); +@@ -751,5 +791,5 @@ + polkit_unix_process_get_owner (PolkitUnixProcess *process, + GError **error) + { +- return _polkit_unix_process_get_owner (process, error); ++ return polkit_unix_process_get_racy_uid__ (process, error); + } +diff -ur polkit/src/polkitbackend/polkitbackendinteractiveauthority.c polkit-0.113/src/polkitbackend/polkitbackendinteractiveauthority.c +--- polkit/src/polkitbackend/polkitbackendinteractiveauthority.c 2018-07-11 18:25:30.714164373 +0200 ++++ polkit-0.113/src/polkitbackend/polkitbackendinteractiveauthority.c 2018-07-11 18:25:38.821870017 +0200 +@@ -572,7 +572,7 @@ + if (polkit_authorization_result_get_is_authorized (result)) + log_result_str = "ALLOWING"; + +- user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, NULL); ++ user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, NULL, NULL); + + subject_str = polkit_subject_to_string (subject); + +@@ -844,6 +844,7 @@ + gchar *subject_str; + PolkitIdentity *user_of_caller; + PolkitIdentity *user_of_subject; ++ gboolean user_of_subject_matches; + gchar *user_of_caller_str; + gchar *user_of_subject_str; + PolkitAuthorizationResult *result; +@@ -889,7 +890,7 @@ + action_id); + + user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, +- caller, ++ caller, NULL, + &error); + if (error != NULL) + { +@@ -904,7 +905,7 @@ + g_debug (" user of caller is %s", user_of_caller_str); + + user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, +- subject, ++ subject, &user_of_subject_matches, + &error); + if (error != NULL) + { +@@ -934,7 +935,10 @@ + * We only allow this if, and only if, + * + * - processes may check for another process owned by the *same* user but not +- * if details are passed (otherwise you'd be able to spoof the dialog) ++ * if details are passed (otherwise you'd be able to spoof the dialog); ++ * the caller supplies the user_of_subject value, so we additionally ++ * require it to match at least at one point in time (via ++ * user_of_subject_matches). + * + * - processes running as uid 0 may check anything and pass any details + * +@@ -942,7 +946,9 @@ + * then any uid referenced by that annotation is also allowed to check + * to check anything and pass any details + */ +- if (!polkit_identity_equal (user_of_caller, user_of_subject) || has_details) ++ if (!user_of_subject_matches ++ || !polkit_identity_equal (user_of_caller, user_of_subject) ++ || has_details) + { + if (!may_identity_check_authorization (interactive_authority, action_id, user_of_caller)) + { +@@ -1107,9 +1113,10 @@ + goto out; + } + +- /* every subject has a user */ ++ /* every subject has a user; this is supplied by the client, so we rely ++ * on the caller to validate its acceptability. */ + user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, +- subject, ++ subject, NULL, + error); + if (user_of_subject == NULL) + goto out; +@@ -2476,6 +2483,7 @@ + PolkitSubject *session_for_caller; + PolkitIdentity *user_of_caller; + PolkitIdentity *user_of_subject; ++ gboolean user_of_subject_matches; + AuthenticationAgent *agent; + gboolean ret; + gchar *caller_cmdline; +@@ -2528,7 +2536,7 @@ + goto out; + } + +- user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, caller, NULL); ++ user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, caller, NULL, NULL); + if (user_of_caller == NULL) + { + g_set_error (error, +@@ -2537,7 +2545,7 @@ + "Cannot determine user of caller"); + goto out; + } +- user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, NULL); ++ user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, &user_of_subject_matches, NULL); + if (user_of_subject == NULL) + { + g_set_error (error, +@@ -2546,7 +2554,8 @@ + "Cannot determine user of subject"); + goto out; + } +- if (!polkit_identity_equal (user_of_caller, user_of_subject)) ++ if (!user_of_subject_matches ++ || !polkit_identity_equal (user_of_caller, user_of_subject)) + { + if (identity_is_root_user (user_of_caller)) + { +@@ -2639,6 +2648,7 @@ + PolkitSubject *session_for_caller; + PolkitIdentity *user_of_caller; + PolkitIdentity *user_of_subject; ++ gboolean user_of_subject_matches; + AuthenticationAgent *agent; + gboolean ret; + gchar *scope_str; +@@ -2687,7 +2697,7 @@ + goto out; + } + +- user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, caller, NULL); ++ user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, caller, NULL, NULL); + if (user_of_caller == NULL) + { + g_set_error (error, +@@ -2696,7 +2706,7 @@ + "Cannot determine user of caller"); + goto out; + } +- user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, NULL); ++ user_of_subject = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, subject, &user_of_subject_matches, NULL); + if (user_of_subject == NULL) + { + g_set_error (error, +@@ -2705,7 +2715,8 @@ + "Cannot determine user of subject"); + goto out; + } +- if (!polkit_identity_equal (user_of_caller, user_of_subject)) ++ if (!user_of_subject_matches ++ || !polkit_identity_equal (user_of_caller, user_of_subject)) + { + if (identity_is_root_user (user_of_caller)) + { +@@ -2815,7 +2826,7 @@ + identity_str); + + user_of_caller = polkit_backend_session_monitor_get_user_for_subject (priv->session_monitor, +- caller, ++ caller, NULL, + error); + if (user_of_caller == NULL) + goto out; +diff -ur polkit/src/polkitbackend/polkitbackendsessionmonitor.c polkit-0.113/src/polkitbackend/polkitbackendsessionmonitor.c +--- polkit/src/polkitbackend/polkitbackendsessionmonitor.c 2015-06-06 01:24:06.000000000 +0200 ++++ polkit-0.113/src/polkitbackend/polkitbackendsessionmonitor.c 2018-07-11 18:25:38.821870017 +0200 +@@ -27,6 +27,7 @@ + #include + + #include ++#include + #include "polkitbackendsessionmonitor.h" + + #define CKDB_PATH "/var/run/ConsoleKit/database" +@@ -273,28 +274,40 @@ + * polkit_backend_session_monitor_get_user: + * @monitor: A #PolkitBackendSessionMonitor. + * @subject: A #PolkitSubject. ++ * @result_matches: If not %NULL, set to indicate whether the return value matches current (RACY) state. + * @error: Return location for error. + * + * Gets the user corresponding to @subject or %NULL if no user exists. + * ++ * NOTE: For a #PolkitUnixProcess, the UID is read from @subject (which may ++ * come from e.g. a D-Bus client), so it may not correspond to the actual UID ++ * of the referenced process (at any point in time). This is indicated by ++ * setting @result_matches to %FALSE; the caller may reject such subjects or ++ * require additional privileges. @result_matches == %TRUE only indicates that ++ * the UID matched the underlying process at ONE point in time, it may not match ++ * later. ++ * + * Returns: %NULL if @error is set otherwise a #PolkitUnixUser that should be freed with g_object_unref(). + */ + PolkitIdentity * + polkit_backend_session_monitor_get_user_for_subject (PolkitBackendSessionMonitor *monitor, + PolkitSubject *subject, ++ gboolean *result_matches, + GError **error) + { + PolkitIdentity *ret; ++ gboolean matches; + GError *local_error; +- gchar *group; +- guint32 uid; + + ret = NULL; ++ matches = FALSE; + + if (POLKIT_IS_UNIX_PROCESS (subject)) + { +- uid = polkit_unix_process_get_uid (POLKIT_UNIX_PROCESS (subject)); +- if ((gint) uid == -1) ++ gint subject_uid, current_uid; ++ ++ subject_uid = polkit_unix_process_get_uid (POLKIT_UNIX_PROCESS (subject)); ++ if (subject_uid == -1) + { + g_set_error (error, + POLKIT_ERROR, +@@ -302,14 +315,26 @@ + "Unix process subject does not have uid set"); + goto out; + } +- ret = polkit_unix_user_new (uid); ++ local_error = NULL; ++ current_uid = polkit_unix_process_get_racy_uid__ (POLKIT_UNIX_PROCESS (subject), &local_error); ++ if (local_error != NULL) ++ { ++ g_propagate_error (error, local_error); ++ goto out; ++ } ++ ret = polkit_unix_user_new (subject_uid); ++ matches = (subject_uid == current_uid); + } + else if (POLKIT_IS_SYSTEM_BUS_NAME (subject)) + { + ret = (PolkitIdentity*)polkit_system_bus_name_get_user_sync (POLKIT_SYSTEM_BUS_NAME (subject), NULL, error); ++ matches = TRUE; + } + else if (POLKIT_IS_UNIX_SESSION (subject)) + { ++ gint uid; ++ gchar *group; ++ + if (!ensure_database (monitor, error)) + { + g_prefix_error (error, "Error getting user for session: Error ensuring CK database at " CKDB_PATH ": "); +@@ -328,9 +353,14 @@ + g_free (group); + + ret = polkit_unix_user_new (uid); ++ matches = TRUE; + } + + out: ++ if (result_matches != NULL) ++ { ++ *result_matches = matches; ++ } + return ret; + } + +diff -ur polkit/src/polkitbackend/polkitbackendsessionmonitor.h polkit-0.113/src/polkitbackend/polkitbackendsessionmonitor.h +--- polkit/src/polkitbackend/polkitbackendsessionmonitor.h 2014-01-14 23:42:25.000000000 +0100 ++++ polkit-0.113/src/polkitbackend/polkitbackendsessionmonitor.h 2018-07-11 18:25:38.822869981 +0200 +@@ -47,6 +47,7 @@ + + PolkitIdentity *polkit_backend_session_monitor_get_user_for_subject (PolkitBackendSessionMonitor *monitor, + PolkitSubject *subject, ++ gboolean *result_matches, + GError **error); + + PolkitSubject *polkit_backend_session_monitor_get_session_for_subject (PolkitBackendSessionMonitor *monitor, +diff -ur polkit/src/polkitbackend/polkitbackendsessionmonitor-systemd.c polkit-0.113/src/polkitbackend/polkitbackendsessionmonitor-systemd.c +--- polkit/src/polkitbackend/polkitbackendsessionmonitor-systemd.c 2015-06-19 22:31:02.000000000 +0200 ++++ polkit-0.113/src/polkitbackend/polkitbackendsessionmonitor-systemd.c 2018-07-11 18:25:38.821870017 +0200 +@@ -29,6 +29,7 @@ + #include + + #include ++#include + #include "polkitbackendsessionmonitor.h" + + /* +@@ -246,26 +247,40 @@ + * polkit_backend_session_monitor_get_user: + * @monitor: A #PolkitBackendSessionMonitor. + * @subject: A #PolkitSubject. ++ * @result_matches: If not %NULL, set to indicate whether the return value matches current (RACY) state. + * @error: Return location for error. + * + * Gets the user corresponding to @subject or %NULL if no user exists. + * ++ * NOTE: For a #PolkitUnixProcess, the UID is read from @subject (which may ++ * come from e.g. a D-Bus client), so it may not correspond to the actual UID ++ * of the referenced process (at any point in time). This is indicated by ++ * setting @result_matches to %FALSE; the caller may reject such subjects or ++ * require additional privileges. @result_matches == %TRUE only indicates that ++ * the UID matched the underlying process at ONE point in time, it may not match ++ * later. ++ * + * Returns: %NULL if @error is set otherwise a #PolkitUnixUser that should be freed with g_object_unref(). + */ + PolkitIdentity * + polkit_backend_session_monitor_get_user_for_subject (PolkitBackendSessionMonitor *monitor, + PolkitSubject *subject, ++ gboolean *result_matches, + GError **error) + { + PolkitIdentity *ret; +- guint32 uid; ++ gboolean matches; + + ret = NULL; ++ matches = FALSE; + + if (POLKIT_IS_UNIX_PROCESS (subject)) + { +- uid = polkit_unix_process_get_uid (POLKIT_UNIX_PROCESS (subject)); +- if ((gint) uid == -1) ++ gint subject_uid, current_uid; ++ GError *local_error; ++ ++ subject_uid = polkit_unix_process_get_uid (POLKIT_UNIX_PROCESS (subject)); ++ if (subject_uid == -1) + { + g_set_error (error, + POLKIT_ERROR, +@@ -273,14 +288,24 @@ + "Unix process subject does not have uid set"); + goto out; + } +- ret = polkit_unix_user_new (uid); ++ local_error = NULL; ++ current_uid = polkit_unix_process_get_racy_uid__ (POLKIT_UNIX_PROCESS (subject), &local_error); ++ if (local_error != NULL) ++ { ++ g_propagate_error (error, local_error); ++ goto out; ++ } ++ ret = polkit_unix_user_new (subject_uid); ++ matches = (subject_uid == current_uid); + } + else if (POLKIT_IS_SYSTEM_BUS_NAME (subject)) + { + ret = (PolkitIdentity*)polkit_system_bus_name_get_user_sync (POLKIT_SYSTEM_BUS_NAME (subject), NULL, error); ++ matches = TRUE; + } + else if (POLKIT_IS_UNIX_SESSION (subject)) + { ++ uid_t uid; + + if (sd_session_get_uid (polkit_unix_session_get_session_id (POLKIT_UNIX_SESSION (subject)), &uid) < 0) + { +@@ -292,9 +317,14 @@ + } + + ret = polkit_unix_user_new (uid); ++ matches = TRUE; + } + + out: ++ if (result_matches != NULL) ++ { ++ *result_matches = matches; ++ } + return ret; + } + diff --git a/polkit.spec b/polkit.spec index 124531e..5ac6c0a 100644 --- a/polkit.spec +++ b/polkit.spec @@ -6,7 +6,7 @@ Summary: An authorization framework Name: polkit Version: 0.113 -Release: 15%{?dist} +Release: 16%{?dist} License: LGPLv2+ URL: http://www.freedesktop.org/wiki/Software/polkit Source0: http://www.freedesktop.org/software/polkit/releases/%{name}-%{version}.tar.gz @@ -17,6 +17,8 @@ Patch0: polkit-0.113-agent-leaks.patch Patch1: polkit-0.113-polkitpermission-leak.patch # already upstream Patch2: polkit-0.113-itstool.patch +# Upstream commit bc7ffad53643a9c80231fc41f5582d6a8931c32c +Patch3: polkit-0.113-CVE-2018-1116.patch Group: System Environment/Libraries BuildRequires: glib2-devel >= 2.30.0 BuildRequires: expat-devel @@ -98,6 +100,7 @@ Libraries files for polkit. %patch0 -p1 -b .agent-leaks %patch1 -p1 -b .polkitpermission-leak %patch2 -p1 -b .itstool +%patch3 -p1 -b .CVE-2018-1116 %build %if 0%{?enable_autoreconf} @@ -185,6 +188,9 @@ exit 0 %{_libdir}/girepository-1.0/*.typelib %changelog +* Wed Jul 11 2018 Miloslav Trmač - 0.113-16 +- Fix CVE-2018-1116 + * Thu Aug 03 2017 Fedora Release Engineering - 0.113-15 - Rebuilt for https://fedoraproject.org/wiki/Fedora_27_Binutils_Mass_Rebuild