[patch] upstreams patch addressing the security issue

user_change_icon_file_authorized_cb-fix-insufficient.patch (text/plain), 2.36 KB, created by Attila Tóth on 2018-07-07 09:56:19 UTC

From 380f4cd7b80176549e91795808d875c1e6e58ccb Mon Sep 17 00:00:00 2001
From: Matthias Gerstner <matthias.gerstner () suse de>
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