Hi Tavis,
On Fri, May 29, 2015, at 02:08 PM, Tavis Ormandy wrote:
> Hello, I've noticed polkitd dumps core if you set an invalid object
> path when calling RegisterAuthenticationAgent. It looks like this code
> doesn't check if error was set before dereferencing it:
Indeed, thanks for the report. Can someone review this patch?
I suppose this'll need a CVE, as local, authenticated users can
can DoS polkitd.
I also updated your test program to properly handle errors,
new version attached.
From 9e074421d5623b6962dc66994d519012b40334b9 Mon Sep 17 00:00:00 2001
From: Colin Walters <walt...@verbum.org>
Date: Sat, 30 May 2015 09:06:23 -0400
Subject: [PATCH] backend: Handle invalid object paths in
RegisterAuthenticationAgent
Properly propagate the error, otherwise we dereference a `NULL`
pointer. This is a local, authenticated DoS.
Reported-by: Tavis Ormandy <tav...@google.com>
Signed-off-by: Colin Walters <walt...@verbum.org>
---
.../polkitbackendinteractiveauthority.c | 53 ++++++++++++----------
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c
index 59028d5..f45fdf1 100644
--- a/src/polkitbackend/polkitbackendinteractiveauthority.c
+++ b/src/polkitbackend/polkitbackendinteractiveauthority.c
@@ -1551,36 +1551,42 @@ authentication_agent_new (PolkitSubject *scope,
const gchar *unique_system_bus_name,
const gchar *locale,
const gchar *object_path,
- GVariant *registration_options)
+ GVariant *registration_options,
+ GError **error)
{
AuthenticationAgent *agent;
- GError *error;
+ GDBusProxy *proxy;
- agent = g_new0 (AuthenticationAgent, 1);
+ if (!g_variant_is_object_path (object_path))
+ {
+ g_set_error (error, POLKIT_ERROR, POLKIT_ERROR_FAILED,
+ "Invalid object path '%s'", object_path);
+ return NULL;
+ }
+
+ proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM,
+ G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES |
+ G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS,
+ NULL, /* GDBusInterfaceInfo* */
+ unique_system_bus_name,
+ object_path,
+ "org.freedesktop.PolicyKit1.AuthenticationAgent",
+ NULL, /* GCancellable* */
+ error);
+ if (proxy == NULL)
+ {
+ g_prefix_error (error, "Failed to construct proxy for agent: " );
+ return NULL;
+ }
+ agent = g_new0 (AuthenticationAgent, 1);
agent->ref_count = 1;
agent->scope = g_object_ref (scope);
agent->object_path = g_strdup (object_path);
agent->unique_system_bus_name = g_strdup (unique_system_bus_name);
agent->locale = g_strdup (locale);
agent->registration_options = registration_options != NULL ? g_variant_ref (registration_options) : NULL;
-
- error = NULL;
- agent->proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM,
- G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES |
- G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS,
- NULL, /* GDBusInterfaceInfo* */
- agent->unique_system_bus_name,
- agent->object_path,
- "org.freedesktop.PolicyKit1.AuthenticationAgent",
- NULL, /* GCancellable* */
- &error);
- if (agent->proxy == NULL)
- {
- g_warning ("Error constructing proxy for agent: %s", error->message);
- g_error_free (error);
- /* TODO: Make authentication_agent_new() return NULL and set a GError */
- }
+ agent->proxy = proxy;
return agent;
}
@@ -2383,8 +2389,6 @@ polkit_backend_interactive_authority_register_authentication_agent (PolkitBacken
caller_cmdline = NULL;
agent = NULL;
- /* TODO: validate that object path is well-formed */
-
interactive_authority = POLKIT_BACKEND_INTERACTIVE_AUTHORITY (authority);
priv = POLKIT_BACKEND_INTERACTIVE_AUTHORITY_GET_PRIVATE (interactive_authority);
@@ -2471,7 +2475,10 @@ polkit_backend_interactive_authority_register_authentication_agent (PolkitBacken
polkit_system_bus_name_get_name (POLKIT_SYSTEM_BUS_NAME (caller)),
locale,
object_path,
- options);
+ options,
+ error);
+ if (!agent)
+ goto out;
g_hash_table_insert (priv->hash_scope_to_authentication_agent,
g_object_ref (subject),
--
1.8.3.1
#define _GNU_SOURCE
#include <gio/gio.h>
#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
int main(int argc, char **argv)
{
GDBusMessage *request;
GDBusMessage *reply;
GDBusConnection *bus;
GVariantBuilder session;
GError *error = NULL;
bus = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL);
request = g_dbus_message_new_method_call("org.freedesktop.PolicyKit1",
"/org/freedesktop/PolicyKit1/Authority",
"org.freedesktop.PolicyKit1.Authority",
"RegisterAuthenticationAgent");
g_variant_builder_init(&session, G_VARIANT_TYPE("a{sv}"));
g_variant_builder_add(&session, "{sv}", "pid",
g_variant_new_uint32(getpid()));
g_variant_builder_add(&session, "{sv}", "start-time",
g_variant_new_uint64(0));
g_dbus_message_set_body(request, g_variant_new("((sa{sv})ss)",
"unix-process", &session, "C", "**invalid**"));
reply = g_dbus_connection_send_message_with_reply_sync(bus, request,
G_DBUS_SEND_MESSAGE_FLAGS_NONE, -1, NULL, NULL, &error);
if (!reply)
goto out;
else
g_print("%s\n", g_dbus_message_print(reply, 0));
out:
if (error)
{
g_printerr("%s\n", error->message);
g_error_free (error);
}
if (reply)
g_object_unref(reply);
g_object_unref(request);
g_variant_builder_clear(&session);
return 0;
}
_______________________________________________
polkit-devel mailing list
polkit-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/polkit-devel
