From 380f4cd7b80176549e91795808d875c1e6e58ccb Mon Sep 17 00:00:00 2001 From: Matthias Gerstner Date: Mon, 2 Jul 2018 12:06:58 +0200 Subject: [PATCH] user_change_icon_file_authorized_cb: fix insufficient path prefix check The path prefix check can be circumvented by regular users by passing relativ path component like so: $ dbus-send --system --print-reply --dest=org.freedesktop.Accounts \ /org/freedesktop/Accounts/User1000 \ org.freedesktop.Accounts.User.SetIconFile \ string:/usr/share/../../tmp/test This results in a user controlled path to be stored in the accountsservice. Clients of accountsservice may trust this path. The existing code actually invests quite some efforts to ensure this by copying the file away with dropped privileges. To fix this situation this commit canonicalized the input path for the prefix comparison. --- src/user.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/user.c b/src/user.c index c8a2942..e1b9a6f 100644 --- a/src/user.c +++ b/src/user.c @@ -1306,6 +1306,7 @@ user_change_icon_file_authorized_cb (Daemon *daemon, { g_autofree gchar *filename = NULL; + g_autofree gchar *canon_filename = NULL; g_autoptr(GFile) file = NULL; g_autoptr(GFileInfo) info = NULL; guint32 mode; @@ -1355,9 +1356,19 @@ user_change_icon_file_authorized_cb (Daemon *daemon, return; } + /* This will not resolve symlinks. But we only want to check for a + * trusted prefix below, so this should be enough. The important bit + * is that no user controlled path is stored in the user data. If + * canon_filename is not trusted then filename will be copied into a + * safe place and the path to the safe place will be stored in the + * user data. + */ + canon_filename = g_file_get_path(file); + if ((mode & S_IROTH) == 0 || - (!g_str_has_prefix (filename, DATADIR) && - !g_str_has_prefix (filename, ICONDIR))) { + !canon_filename || + (!g_str_has_prefix (canon_filename, DATADIR) && + !g_str_has_prefix (canon_filename, ICONDIR))) { g_autofree gchar *dest_path = NULL; g_autoptr(GFile) dest = NULL; const gchar *argv[3]; -- 2.16.4