From 7097ae07a98c03a2a276548cee056153e9394bb4 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Mar 22 2010 14:43:40 +0000 Subject: 0.5 --- diff --git a/.cvsignore b/.cvsignore index 1a225fe..fbda380 100644 --- a/.cvsignore +++ b/.cvsignore @@ -1 +1 @@ -accountsservice-0.4.tar.bz2 +accountsservice-0.5.tar.bz2 diff --git a/0001-Avoid-extraneous-commandline-parsing.patch b/0001-Avoid-extraneous-commandline-parsing.patch new file mode 100644 index 0000000..f669b72 --- /dev/null +++ b/0001-Avoid-extraneous-commandline-parsing.patch @@ -0,0 +1,502 @@ +From 0a4c2503e0a6b1bdb34f3fa8f6644250eccbc445 Mon Sep 17 00:00:00 2001 +From: Matthias Clasen +Date: Fri, 19 Mar 2010 19:28:56 -0400 +Subject: [PATCH] Avoid extraneous commandline parsing + +Switch to using g_spawn_sync instead of g_spawn_command_line_sync +to avoid the pointless roundtrip through a commandline parser, avoiding +some security pitfalls. +--- + src/daemon.c | 46 ++++++++++++++++-------- + src/user.c | 115 ++++++++++++++++++++++++++++++++++------------------------ + 2 files changed, 98 insertions(+), 63 deletions(-) + +diff --git a/src/daemon.c b/src/daemon.c +index a962dc3..18a23fe 100644 +--- a/src/daemon.c ++++ b/src/daemon.c +@@ -922,7 +922,7 @@ typedef struct { + DBusGMethodInvocation *context; + } ListUserData; + +-ListUserData * ++static ListUserData * + list_user_data_new (Daemon *daemon, + DBusGMethodInvocation *context) + { +@@ -1006,11 +1006,10 @@ daemon_create_user_authorized_cb (Daemon *daemon, + { + CreateUserData *cd = data; + User *user; +- gchar *cmdline; + GError *error; + gchar *std_err, *std_out; + gint status; +- const gchar *grouparg; ++ gchar *argv[8]; + + if (getpwnam (cd->user_name) != NULL) { + throw_error (context, ERROR_USER_EXISTS, "A user with name '%s' already exists", cd->user_name); +@@ -1022,22 +1021,32 @@ daemon_create_user_authorized_cb (Daemon *daemon, + "create user '%s'", + cd->user_name); + ++ argv[0] = "/usr/sbin/useradd"; ++ argv[1] = "-m"; ++ argv[2] = "-c"; ++ argv[3] = cd->real_name; + if (cd->account_type == ACCOUNT_TYPE_ADMINISTRATOR) { +- grouparg = "-G desktop_admin_r"; ++ argv[4] = "-G"; ++ argv[5] = "desktop_admin_r"; ++ argv[6] = cd->user_name; ++ argv[7] = NULL; + } + else if (cd->account_type == ACCOUNT_TYPE_STANDARD) { +- grouparg = "-G desktop_user_r"; ++ argv[4] = "-G"; ++ argv[5] = "desktop_user_r"; ++ argv[6] = cd->user_name; ++ argv[7] = NULL; + } + else { +- grouparg = ""; ++ argv[4] = cd->user_name; ++ argv[5] = NULL; + } +- cmdline = g_strdup_printf ("/usr/sbin/useradd -m -c '%s' %s %s", cd->real_name, grouparg, cd->user_name); + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -1051,7 +1060,6 @@ daemon_create_user_authorized_cb (Daemon *daemon, + return; + } + +- g_free (cmdline); + g_free (std_out); + g_free (std_err); + +@@ -1099,12 +1107,12 @@ daemon_delete_user_authorized_cb (Daemon *daemon, + + { + DeleteUserData *ud = data; +- gchar *cmdline; + GError *error; + gchar *std_err, *std_out; + gint status; + gchar *filename; + struct passwd *pwent; ++ gchar *argv[4]; + + pwent = getpwuid (ud->uid); + +@@ -1118,13 +1126,22 @@ daemon_delete_user_authorized_cb (Daemon *daemon, + "delete user '%s' (%d)", + pwent->pw_name, ud->uid); + +- cmdline = g_strdup_printf ("/usr/sbin/userdel %s%s", ud->remove_files ? "-r " : "", pwent->pw_name); ++ argv[0] = "/usr/sbin/userdel"; ++ if (ud->remove_files) { ++ argv[1] = "-r"; ++ argv[2] = pwent->pw_name; ++ argv[3] = NULL; ++ } ++ else { ++ argv[1] = pwent->pw_name; ++ argv[2] = NULL; ++ } + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -1142,7 +1159,6 @@ daemon_delete_user_authorized_cb (Daemon *daemon, + g_remove (filename); + + g_free (filename); +- g_free (cmdline); + + dbus_g_method_return (context); + } +diff --git a/src/user.c b/src/user.c +index a235411..cfdd16c 100644 +--- a/src/user.c ++++ b/src/user.c +@@ -737,20 +737,24 @@ user_change_real_name_authorized_cb (Daemon *daemon, + GError *error; + gint status; + gchar *std_out, *std_err; +- gchar *cmdline; ++ gchar *argv[5]; + + if (g_strcmp0 (user->real_name, name) != 0) { + daemon_local_log (daemon, context, + "change real name of user '%s' (%d) to '%s'", + user->user_name, user->uid, name); + +- cmdline = g_strdup_printf ("/usr/sbin/usermod -c '%s' %s", name, user->user_name); ++ argv[0] = "/usr/sbin/usermod"; ++ argv[1] = "-c"; ++ argv[2] = name; ++ argv[3] = user->user_name; ++ argv[4] = NULL; + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -761,13 +765,11 @@ user_change_real_name_authorized_cb (Daemon *daemon, + throw_error (context, ERROR_FAILED, "usermod returned an error: %s", std_err); + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + return; + } + + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + + g_free (user->real_name); + user->real_name = g_strdup (name); +@@ -831,7 +833,7 @@ user_change_user_name_authorized_cb (Daemon *daemon, + GError *error; + gint status; + gchar *std_out, *std_err; +- gchar *cmdline; ++ gchar *argv[5]; + + if (g_strcmp0 (user->user_name, name) != 0) { + old_name = g_strdup (user->user_name); +@@ -839,13 +841,17 @@ user_change_user_name_authorized_cb (Daemon *daemon, + "change name of user '%s' (%d) to '%s'", + old_name, user->uid, name); + +- cmdline = g_strdup_printf ("/usr/sbin/usermod -l %s %s", name, user->user_name); ++ argv[0] = "/usr/sbin/usermod"; ++ argv[1] = "-l"; ++ argv[2] = name; ++ argv[3] = user->user_name; ++ argv[4] = NULL; + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -856,13 +862,11 @@ user_change_user_name_authorized_cb (Daemon *daemon, + throw_error (context, ERROR_FAILED, "usermod returned an error: %s", std_err); + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + return; + } + + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + + g_free (user->user_name); + user->user_name = g_strdup (name); +@@ -1118,19 +1122,25 @@ user_change_home_dir_authorized_cb (Daemon *daemon, + GError *error; + gint status; + gchar *std_out, *std_err; +- gchar *cmdline; ++ gchar *argv[6]; + + if (g_strcmp0 (user->home_dir, home_dir) != 0) { + daemon_local_log (daemon, context, + "change home directory of user '%s' (%d) to '%s'", + user->user_name, user->uid, home_dir); +- cmdline = g_strdup_printf ("/usr/sbin/usermod -m -d '%s' %s", home_dir, user->user_name); ++ ++ argv[0] = "/usr/sbin/usermod"; ++ argv[1] = "-m"; ++ argv[2] = "-d"; ++ argv[3] = home_dir; ++ argv[4] = user->user_name; ++ argv[5] = NULL; + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -1141,13 +1151,11 @@ user_change_home_dir_authorized_cb (Daemon *daemon, + throw_error (context, ERROR_FAILED, "usermod returned an error: %s", std_err); + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + return; + } + + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + + g_free (user->home_dir); + user->home_dir = g_strdup (home_dir); +@@ -1159,6 +1167,7 @@ user_change_home_dir_authorized_cb (Daemon *daemon, + + dbus_g_method_return (context); + } ++ + gboolean + user_set_home_directory (User *user, + const gchar *home_dir, +@@ -1206,20 +1215,24 @@ user_change_shell_authorized_cb (Daemon *daemon, + GError *error; + gint status; + gchar *std_out, *std_err; +- gchar *cmdline; ++ gchar *argv[5]; + + if (g_strcmp0 (user->shell, shell) != 0) { + daemon_local_log (daemon, context, + "change shell of user '%s' (%d) to '%s'", + user->user_name, user->uid, shell); + +- cmdline = g_strdup_printf ("/usr/sbin/usermod -s '%s' %s", shell, user->user_name); ++ argv[0] = "/usr/sbin/usermod"; ++ argv[1] = "-s"; ++ argv[2] = shell; ++ argv[3] = user->user_name; ++ argv[4] = NULL; + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -1230,13 +1243,11 @@ user_change_shell_authorized_cb (Daemon *daemon, + throw_error (context, ERROR_FAILED, "usermod returned an error: %s", std_err); + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + return; + } + + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + + g_free (user->shell); + user->shell = g_strdup (shell); +@@ -1475,19 +1486,22 @@ user_change_locked_authorized_cb (Daemon *daemon, + GError *error; + gint status; + gchar *std_out, *std_err; +- gchar *cmdline; ++ gchar *argv[4]; + + if (user->locked != locked) { + daemon_local_log (daemon, context, + "%s account of user '%s' (%d)", + locked ? "locking" : "unlocking", user->user_name, user->uid); +- cmdline = g_strdup_printf ("/usr/sbin/usermod -%c %s", locked ? 'L' : 'U', user->user_name); ++ argv[0] = "/usr/sbin/usermod"; ++ argv[1] = locked ? "-L" : "-U"; ++ argv[2] = user->user_name; ++ argv[3] = NULL; + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -1498,13 +1512,11 @@ user_change_locked_authorized_cb (Daemon *daemon, + throw_error (context, ERROR_FAILED, "usermod returned an error: %s", std_err); + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + return; + } + + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + + user->locked = locked; + +@@ -1544,7 +1556,6 @@ user_change_account_type_authorized_cb (Daemon *daemon, + GError *error; + gint status; + gchar *std_out, *std_err; +- gchar *cmdline; + gid_t groups[20]; + gint n_groups; + GString *str; +@@ -1552,6 +1563,7 @@ user_change_account_type_authorized_cb (Daemon *daemon, + gid_t desktop_admin_r; + struct group *grp; + gint i; ++ gchar *argv[5]; + + if (user->account_type != account_type) { + daemon_local_log (daemon, context, +@@ -1596,13 +1608,19 @@ user_change_account_type_authorized_cb (Daemon *daemon, + g_string_truncate (str, str->len - 1); + } + +- cmdline = g_strdup_printf ("/usr/sbin/usermod -G %s %s", str->str, user->user_name); ++ argv[0] = "/usr/sbin/usermod"; ++ argv[1] = "-G"; ++ argv[2] = str->str; ++ argv[3] = user->user_name; ++ argv[4] = NULL; ++ ++ g_string_free (str, FALSE); + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -1613,13 +1631,11 @@ user_change_account_type_authorized_cb (Daemon *daemon, + throw_error (context, ERROR_FAILED, "usermod returned an error: %s", std_err); + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + return; + } + + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + + user->account_type = account_type; + +@@ -1664,7 +1680,7 @@ user_change_password_mode_authorized_cb (Daemon *daemon, + GError *error; + gint status; + gchar *std_out, *std_err; +- gchar *cmdline; ++ gchar *argv[4]; + + if (user->password_mode != mode) { + daemon_local_log (daemon, context, +@@ -1674,13 +1690,16 @@ user_change_password_mode_authorized_cb (Daemon *daemon, + if (mode == PASSWORD_MODE_SET_AT_LOGIN || + mode == PASSWORD_MODE_NONE) { + +- cmdline = g_strdup_printf ("/usr/bin/passwd -d %s", user->user_name); ++ argv[0] = "/usr/bin/passwd"; ++ argv[1] = "-d"; ++ argv[2] = user->user_name; ++ argv[3] = NULL; + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -1691,13 +1710,11 @@ user_change_password_mode_authorized_cb (Daemon *daemon, + throw_error (context, ERROR_FAILED, "usermod returned an error: %s", std_err); + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + return; + } + + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + + g_free (user->password_hint); + user->password_hint = NULL; +@@ -1774,19 +1791,23 @@ user_change_password_authorized_cb (Daemon *daemon, + GError *error; + gint status; + gchar *std_out, *std_err; +- gchar *cmdline; ++ gchar *argv[5]; + + daemon_local_log (daemon, context, + "set password and hint of user '%s' (%d)", + user->user_name, user->uid); + +- cmdline = g_strdup_printf ("/usr/sbin/usermod -p '%s' %s", strings[0], user->user_name); ++ argv[0] = "/usr/sbin/usermod"; ++ argv[1] = "-p"; ++ argv[2] = strings[0]; ++ argv[3] = user->user_name; ++ argv[4] = NULL; + + std_out = NULL; + std_err = NULL; + error = NULL; +- if (!g_spawn_command_line_sync (cmdline, &std_out, &std_err, &status, &error)) { +- throw_error (context, ERROR_FAILED, "running '%s' failed: %s", cmdline, error->message); ++ if (!g_spawn_sync (NULL, argv, NULL, 0, NULL, NULL, &std_out, &std_err, &status, &error)) { ++ throw_error (context, ERROR_FAILED, "running '%s' failed: %s", argv[0], error->message); + g_error_free (error); + g_free (std_out); + g_free (std_err); +@@ -1797,13 +1818,11 @@ user_change_password_authorized_cb (Daemon *daemon, + throw_error (context, ERROR_FAILED, "usermod returned an error: %s", std_err); + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + return; + } + + g_free (std_out); + g_free (std_err); +- g_free (cmdline); + + if (user->password_mode != PASSWORD_MODE_REGULAR) { + user->password_mode = PASSWORD_MODE_REGULAR; +-- +1.7.0.1 + diff --git a/accountsservice.spec b/accountsservice.spec index aceb034..b276ce9 100644 --- a/accountsservice.spec +++ b/accountsservice.spec @@ -1,13 +1,13 @@ Name: accountsservice -Version: 0.4 -Release: 3%{?dist} +Version: 0.5 +Release: 1%{?dist} Summary: D-Bus interfaces for querying and manipulating user account information Group: System Environment/Daemons License: GPLv3+ URL: http://www.fedoraproject.org/wiki/Features/UserAccountDialog -Source0: http://mclasen.fedorapeople.org/accounts/accountsservice-0.4.tar.bz2 +Source0: http://mclasen.fedorapeople.org/accounts/accountsservice-0.5.tar.bz2 BuildRequires: glib2-devel BuildRequires: gtk2-devel @@ -56,6 +56,9 @@ rm -rf $RPM_BUILD_ROOT %changelog +* Mon Mar 22 2010 Matthias Clasen 0.5-1 +- Update to 0.5 + * Mon Feb 22 2010 Bastien Nocera 0.4-3 - Fix directory ownership diff --git a/sources b/sources index ade62c2..7d3af2c 100644 --- a/sources +++ b/sources @@ -1 +1 @@ -cb2ca0e1b45873fdd80fa7d8aeef7eac accountsservice-0.4.tar.bz2 +7562b59d6bebae6bafa4eb56d413f6f6 accountsservice-0.5.tar.bz2