Properly validate all arguments passed via D-Bus

4 files changed, 92 insertions, 26 deletions

diff --git a/docs/TODO b/docs/TODO
index 6bf596e..5cd211d 100644
--- a/docs/TODO
+++ b/docs/TODO
@@ -7,11 +7,6 @@ Needed for 1.0
 
  - man page review / section review
 
- - check / validate all incoming arguments
-   - includes all D-Bus interfaces and public library API
-   - validate D-Bus object paths everywhere
-   - ...and other security/paranoia stuff
-
  - make sure library API is reasonably MT-safe
 
  - avoid watching all name owner changes in PolkitBackendAuthority and
diff --git a/src/polkit/polkitidentity.c b/src/polkit/polkitidentity.c
index e1b14d6..6e33136 100644
--- a/src/polkit/polkitidentity.c
+++ b/src/polkit/polkitidentity.c
@@ -201,12 +201,14 @@ polkit_identity_new_for_real (_PolkitIdentity *real)
   if (strcmp (kind, "unix-user") == 0)
     {
       variant = egg_dbus_hash_map_lookup (details, "uid");
-      s = polkit_unix_user_new (egg_dbus_variant_get_uint (variant));
+      if (variant != NULL)
+        s = polkit_unix_user_new (egg_dbus_variant_get_uint (variant));
     }
   else if (strcmp (kind, "unix-group") == 0)
     {
       variant = egg_dbus_hash_map_lookup (details, "gid");
-      s = polkit_unix_group_new (egg_dbus_variant_get_uint (variant));
+      if (variant != NULL)
+        s = polkit_unix_group_new (egg_dbus_variant_get_uint (variant));
     }
   else
     {
diff --git a/src/polkit/polkitsubject.c b/src/polkit/polkitsubject.c
index 04067da..d5039a5 100644
--- a/src/polkit/polkitsubject.c
+++ b/src/polkit/polkitsubject.c
@@ -282,18 +282,21 @@ polkit_subject_new_for_real (_PolkitSubject *real)
     {
       variant = egg_dbus_hash_map_lookup (details, "pid");
       variant2 = egg_dbus_hash_map_lookup (details, "start-time");
-      s = polkit_unix_process_new_full (egg_dbus_variant_get_uint (variant),
-                                        egg_dbus_variant_get_uint64 (variant2));
+      if (variant != NULL && variant2 != NULL)
+        s = polkit_unix_process_new_full (egg_dbus_variant_get_uint (variant),
+                                          egg_dbus_variant_get_uint64 (variant2));
     }
   else if (strcmp (kind, "unix-session") == 0)
     {
       variant = egg_dbus_hash_map_lookup (details, "session-id");
-      s = polkit_unix_session_new (egg_dbus_variant_get_string (variant));
+      if (variant != NULL)
+        s = polkit_unix_session_new (egg_dbus_variant_get_string (variant));
     }
   else if (strcmp (kind, "system-bus-name") == 0)
     {
       variant = egg_dbus_hash_map_lookup (details, "name");
-      s = polkit_system_bus_name_new (egg_dbus_variant_get_string (variant));
+      if (variant != NULL)
+        s = polkit_system_bus_name_new (egg_dbus_variant_get_string (variant));
     }
   else
     {
diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c
index 4ed97e3..090e350 100644
--- a/src/polkitbackend/polkitbackendauthority.c
+++ b/src/polkitbackend/polkitbackendauthority.c
@@ -899,10 +899,20 @@ authority_handle_check_authorization (_PolkitAuthority               *instance,
   GCancellable *cancellable;
   PolkitDetails *details;
 
-  caller_name = egg_dbus_method_invocation_get_caller (method_invocation);
-  caller = polkit_system_bus_name_new (caller_name);
+  details = NULL;
 
   subject = polkit_subject_new_for_real (real_subject);
+  if (subject == NULL)
+    {
+      egg_dbus_method_invocation_return_error_literal (method_invocation,
+                                                       _POLKIT_ERROR,
+                                                       _POLKIT_ERROR_FAILED,
+                                                       "Error parsing subject struct");
+      goto out;
+    }
+
+  caller_name = egg_dbus_method_invocation_get_caller (method_invocation);
+  caller = polkit_system_bus_name_new (caller_name);
 
   details = polkit_details_new_for_hash (real_details->data);
 
@@ -948,7 +958,8 @@ authority_handle_check_authorization (_PolkitAuthority               *instance,
                                                 check_auth_cb,
                                                 method_invocation);
  out:
-  g_object_unref (details);
+  if (details != NULL)
+    g_object_unref (details);
 }
 
 static void
@@ -999,10 +1010,21 @@ authority_handle_register_authentication_agent (_PolkitAuthority               *
   PolkitSubject *subject;
   GError *error;
 
-  caller = polkit_system_bus_name_new (egg_dbus_method_invocation_get_caller (method_invocation));
+  caller = NULL;
+
   subject = polkit_subject_new_for_real (real_subject);
+  if (subject == NULL)
+    {
+      egg_dbus_method_invocation_return_error_literal (method_invocation,
+                                                       _POLKIT_ERROR,
+                                                       _POLKIT_ERROR_FAILED,
+                                                       "Error parsing subject struct");
+      goto out;
+    }
   g_object_set_data_full (G_OBJECT (method_invocation), "subject", subject, (GDestroyNotify) g_object_unref);
 
+  caller = polkit_system_bus_name_new (egg_dbus_method_invocation_get_caller (method_invocation));
+
   error = NULL;
   if (!polkit_backend_authority_register_authentication_agent (server->authority,
                                                                caller,
@@ -1019,7 +1041,8 @@ authority_handle_register_authentication_agent (_PolkitAuthority               *
   _polkit_authority_handle_register_authentication_agent_finish (method_invocation);
 
  out:
-  g_object_unref (caller);
+  if (caller != NULL)
+    g_object_unref (caller);
 }
 
 /* ---------------------------------------------------------------------------------------------------- */
@@ -1035,10 +1058,21 @@ authority_handle_unregister_authentication_agent (_PolkitAuthority
   PolkitSubject *subject;
   GError *error;
 
-  caller = polkit_system_bus_name_new (egg_dbus_method_invocation_get_caller (method_invocation));
+  caller = NULL;
+
   subject = polkit_subject_new_for_real (real_subject);
+  if (subject == NULL)
+    {
+      egg_dbus_method_invocation_return_error_literal (method_invocation,
+                                                       _POLKIT_ERROR,
+                                                       _POLKIT_ERROR_FAILED,
+                                                       "Error parsing subject struct");
+      goto out;
+    }
   g_object_set_data_full (G_OBJECT (method_invocation), "subject", subject, (GDestroyNotify) g_object_unref);
 
+  caller = polkit_system_bus_name_new (egg_dbus_method_invocation_get_caller (method_invocation));
+
   error = NULL;
   if (!polkit_backend_authority_unregister_authentication_agent (server->authority,
                                                                  caller,
@@ -1054,7 +1088,8 @@ authority_handle_unregister_authentication_agent (_PolkitAuthority
   _polkit_authority_handle_unregister_authentication_agent_finish (method_invocation);
 
  out:
-  g_object_unref (caller);
+  if (caller != NULL)
+    g_object_unref (caller);
 }
 
 /* ---------------------------------------------------------------------------------------------------- */
@@ -1070,7 +1105,18 @@ authority_handle_authentication_agent_response (_PolkitAuthority               *
   PolkitIdentity *identity;
   GError *error;
 
+  caller = NULL;
+  identity = NULL;
+
   identity = polkit_identity_new_for_real (real_identity);
+  if (identity == NULL)
+    {
+      egg_dbus_method_invocation_return_error_literal (method_invocation,
+                                                       _POLKIT_ERROR,
+                                                       _POLKIT_ERROR_FAILED,
+                                                       "Error parsing identity struct");
+      goto out;
+    }
 
   caller = polkit_system_bus_name_new (egg_dbus_method_invocation_get_caller (method_invocation));
 
@@ -1089,9 +1135,11 @@ authority_handle_authentication_agent_response (_PolkitAuthority               *
   _polkit_authority_handle_authentication_agent_response_finish (method_invocation);
 
  out:
-  g_object_unref (caller);
+  if (caller != NULL)
+    g_object_unref (caller);
 
-  g_object_unref (identity);
+  if (identity != NULL)
+    g_object_unref (identity);
 }
 
 /* ---------------------------------------------------------------------------------------------------- */
@@ -1113,11 +1161,19 @@ authority_handle_enumerate_temporary_authorizations (_PolkitAuthority        *in
   caller = NULL;
   temporary_authorizations = NULL;
 
-  caller = polkit_system_bus_name_new (egg_dbus_method_invocation_get_caller (method_invocation));
-
   subject = polkit_subject_new_for_real (real_subject);
+  if (subject == NULL)
+    {
+      egg_dbus_method_invocation_return_error_literal (method_invocation,
+                                                       _POLKIT_ERROR,
+                                                       _POLKIT_ERROR_FAILED,
+                                                       "Error parsing subject struct");
+      goto out;
+    }
   g_object_set_data_full (G_OBJECT (method_invocation), "subject", subject, (GDestroyNotify) g_object_unref);
 
+  caller = polkit_system_bus_name_new (egg_dbus_method_invocation_get_caller (method_invocation));
+
   temporary_authorizations = polkit_backend_authority_enumerate_temporary_authorizations (server->authority,
                                                                                           caller,
                                                                                           subject,
@@ -1150,7 +1206,8 @@ authority_handle_enumerate_temporary_authorizations (_PolkitAuthority        *in
  out:
   g_list_foreach (temporary_authorizations, (GFunc) g_object_unref, NULL);
   g_list_free (temporary_authorizations);
-  g_object_unref (caller);
+  if (caller != NULL)
+    g_object_unref (caller);
 }
 
 /* ---------------------------------------------------------------------------------------------------- */
@@ -1168,11 +1225,19 @@ authority_handle_revoke_temporary_authorizations (_PolkitAuthority        *insta
   error = NULL;
   caller = NULL;
 
-  caller = polkit_system_bus_name_new (egg_dbus_method_invocation_get_caller (method_invocation));
-
   subject = polkit_subject_new_for_real (real_subject);
+  if (subject == NULL)
+    {
+      egg_dbus_method_invocation_return_error_literal (method_invocation,
+                                                       _POLKIT_ERROR,
+                                                       _POLKIT_ERROR_FAILED,
+                                                       "Error parsing subject struct");
+      goto out;
+    }
   g_object_set_data_full (G_OBJECT (method_invocation), "subject", subject, (GDestroyNotify) g_object_unref);
 
+  caller = polkit_system_bus_name_new (egg_dbus_method_invocation_get_caller (method_invocation));
+
   polkit_backend_authority_revoke_temporary_authorizations (server->authority,
                                                             caller,
                                                             subject,
@@ -1187,7 +1252,8 @@ authority_handle_revoke_temporary_authorizations (_PolkitAuthority        *insta
   _polkit_authority_handle_revoke_temporary_authorizations_finish (method_invocation);
 
  out:
-  g_object_unref (caller);
+  if (caller != NULL)
+    g_object_unref (caller);
 }
 
 /* ---------------------------------------------------------------------------------------------------- */