Fix a bunch of issues when getting/putting a file from cups

There was basically no check for permissions. Now, we temporarily change
our effective uid/gid to the one of the user to open the file for
writing (when getting) or reading (when putting). We then only use
operations that work on the file descriptor to avoid potential race
conditions.

Before that, people could:
 - overwrite any file with the content of a cups resource
 - put any file in a cups resource

Part of fix for CVE-2012-4510.

3 files changed, 183 insertions, 36 deletions

diff --git a/src/cups-pk-helper-mechanism.c b/src/cups-pk-helper-mechanism.c
index d456499..a468a2a 100644
--- a/src/cups-pk-helper-mechanism.c
+++ b/src/cups-pk-helper-mechanism.c
@@ -559,14 +559,28 @@ cph_mechanism_file_get (CphIfaceMechanism     *object,
                         const char            *filename)
 {
         CphMechanism *mechanism = CPH_MECHANISM (object);
+        unsigned int  sender_uid;
         gboolean      ret;
 
         _cph_mechanism_emit_called (mechanism);
 
+        if (!_cph_mechanism_get_sender_uid (mechanism, context, &sender_uid)) {
+                GError *error;
+
+                error = g_error_new (CPH_MECHANISM_ERROR,
+                                     CPH_MECHANISM_ERROR_GENERAL,
+                                     "Cannot determine sender UID");
+                g_dbus_method_invocation_return_gerror (context, error);
+                g_error_free (error);
+
+                return TRUE;
+        }
+
         if (!_check_polkit_for_action (mechanism, context, "server-settings"))
                 return TRUE;
 
-        ret = cph_cups_file_get (mechanism->priv->cups, resource, filename);
+        ret = cph_cups_file_get (mechanism->priv->cups,
+                                 resource, filename, sender_uid);
 
         cph_iface_mechanism_complete_file_get (
                         object, context,
@@ -581,14 +595,28 @@ cph_mechanism_file_put (CphIfaceMechanism     *object,
                         const char            *filename)
 {
         CphMechanism *mechanism = CPH_MECHANISM (object);
+        unsigned int  sender_uid;
         gboolean      ret;
 
         _cph_mechanism_emit_called (mechanism);
 
+        if (!_cph_mechanism_get_sender_uid (mechanism, context, &sender_uid)) {
+                GError *error;
+
+                error = g_error_new (CPH_MECHANISM_ERROR,
+                                     CPH_MECHANISM_ERROR_GENERAL,
+                                     "Cannot determine sender UID");
+                g_dbus_method_invocation_return_gerror (context, error);
+                g_error_free (error);
+
+                return TRUE;
+        }
+
         if (!_check_polkit_for_action (mechanism, context, "server-settings"))
                 return TRUE;
 
-        ret = cph_cups_file_put (mechanism->priv->cups, resource, filename);
+        ret = cph_cups_file_put (mechanism->priv->cups,
+                                 resource, filename, sender_uid);
 
         cph_iface_mechanism_complete_file_put (
                         object, context,
diff --git a/src/cups.c b/src/cups.c
index 7d46c96..6ff6cba 100644
--- a/src/cups.c
+++ b/src/cups.c
@@ -28,6 +28,7 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <ctype.h>
+#include <pwd.h>
 #include <sys/wait.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -510,6 +511,34 @@ _CPH_CUPS_IS_VALID (filename, "filename", TRUE, CPH_STR_MAXLEN)
  * Helpers
  ******************************************************/
 
+static gboolean
+_cph_cups_set_effective_id (unsigned int sender_uid)
+{
+        struct passwd *password_entry;
+
+        password_entry = getpwuid ((uid_t) sender_uid);
+
+        if (password_entry == NULL ||
+            setegid (password_entry->pw_gid) != 0)
+                return FALSE;
+
+        if (seteuid (sender_uid) != 0) {
+                if (getgid () != getegid ())
+                        setegid (getgid ());
+
+                return FALSE;
+        }
+
+        return TRUE;
+}
+
+static void
+_cph_cups_reset_effective_id (void)
+{
+        seteuid (getuid ());
+        setegid (getgid ());
+}
+
 static void
 _cph_cups_add_printer_uri (ipp_t      *request,
                            const char *name)
@@ -1081,14 +1110,15 @@ cph_cups_is_printer_local (CphCups    *cups,
 }
 
 gboolean
-cph_cups_file_get (CphCups    *cups,
-                   const char *resource,
-                   const char *filename)
+cph_cups_file_get (CphCups      *cups,
+                   const char   *resource,
+                   const char   *filename,
+                   unsigned int  sender_uid)
 {
         http_status_t status;
+        int           fd;
         struct stat   file_stat;
-        uid_t         uid;
-        gid_t         gid;
+        char         *error;
 
         g_return_val_if_fail (CPH_IS_CUPS (cups), FALSE);
 
@@ -1097,44 +1127,83 @@ cph_cups_file_get (CphCups    *cups,
         if (!_cph_cups_is_filename_valid (cups, filename))
                 return FALSE;
 
-        stat (filename, &file_stat);
-        uid = file_stat.st_uid;
-        gid = file_stat.st_gid;
+        if (!_cph_cups_set_effective_id (sender_uid)) {
+                error = g_strdup_printf ("Cannot check if \"%s\" is "
+                                         "writable: %s",
+                                         filename, strerror (errno));
+                _cph_cups_set_internal_status (cups, error);
+                g_free (error);
+
+                return FALSE;
+        }
+
+        fd = open (filename, O_WRONLY | O_NOFOLLOW | O_TRUNC);
+
+        _cph_cups_reset_effective_id ();
+
+        if (fd < 0) {
+                error = g_strdup_printf ("Cannot open \"%s\": %s",
+                                         filename, strerror (errno));
+                _cph_cups_set_internal_status (cups, error);
+                g_free (error);
+
+                return FALSE;
+        }
+
+
+        if (fstat (fd, &file_stat) != 0) {
+                error = g_strdup_printf ("Cannot write to \"%s\": %s",
+                                         filename, strerror (errno));
+                _cph_cups_set_internal_status (cups, error);
+                g_free (error);
+
+                close (fd);
+
+                return FALSE;
+        }
+
+        if (!S_ISREG (file_stat.st_mode)) {
+                /* hrm, this looks suspicious... we won't help */
+                error = g_strdup_printf ("File \"%s\" is not a regular file.",
+                                         filename);
+                _cph_cups_set_internal_status (cups, error);
+                g_free (error);
+
+                close (fd);
+
+                return FALSE;
+        }
 
         /* reset the internal status: we'll use the http status */
         _cph_cups_set_internal_status (cups, NULL);
 
-        status = cupsGetFile (cups->priv->connection, resource, filename);
+        status = cupsGetFd (cups->priv->connection, resource, fd);
 
         /* FIXME: There's a bug where the cups connection can fail with EPIPE.
-         * We're work-arounding it here until it's fixed in cups. */
+         * We're working around it here until it's fixed in cups. */
         if (status != HTTP_OK) {
-                if (cph_cups_reconnect (cups)) {
-                        int fd;
-
-                        /* if cupsGetFile fail, then filename is unlinked */
-                        fd = open (filename, O_CREAT, S_IRUSR | S_IWUSR);
-                        close (fd);
-                        chown (filename, uid, gid);
-
-                        _cph_cups_set_internal_status (cups, NULL);
-
-                        status = cupsGetFile (cups->priv->connection,
-                                              resource, filename);
-                }
+                if (cph_cups_reconnect (cups))
+                        status = cupsGetFd (cups->priv->connection,
+                                            resource, fd);
         }
 
+        close (fd);
+
         _cph_cups_set_internal_status_from_http (cups, status);
 
         return (status == HTTP_OK);
 }
 
 gboolean
-cph_cups_file_put (CphCups    *cups,
-                   const char *resource,
-                   const char *filename)
+cph_cups_file_put (CphCups      *cups,
+                   const char   *resource,
+                   const char   *filename,
+                   unsigned int  sender_uid)
 {
         http_status_t status;
+        int           fd;
+        struct stat   file_stat;
+        char         *error;
 
         g_return_val_if_fail (CPH_IS_CUPS (cups), FALSE);
 
@@ -1143,10 +1212,58 @@ cph_cups_file_put (CphCups    *cups,
         if (!_cph_cups_is_filename_valid (cups, filename))
                 return FALSE;
 
+        if (!_cph_cups_set_effective_id (sender_uid)) {
+                error = g_strdup_printf ("Cannot check if \"%s\" is "
+                                         "readable: %s",
+                                         filename, strerror (errno));
+                _cph_cups_set_internal_status (cups, error);
+                g_free (error);
+
+                return FALSE;
+        }
+
+        fd = open (filename, O_RDONLY);
+
+        _cph_cups_reset_effective_id ();
+
+        if (fd < 0) {
+                error = g_strdup_printf ("Cannot open \"%s\": %s",
+                                         filename, strerror (errno));
+                _cph_cups_set_internal_status (cups, error);
+                g_free (error);
+
+                return FALSE;
+        }
+
+        if (fstat (fd, &file_stat) != 0) {
+                error = g_strdup_printf ("Cannot read \"%s\": %s",
+                                         filename, strerror (errno));
+                _cph_cups_set_internal_status (cups, error);
+                g_free (error);
+
+                close (fd);
+
+                return FALSE;
+        }
+
+        if (!S_ISREG (file_stat.st_mode)) {
+                /* hrm, this looks suspicious... we won't help */
+                error = g_strdup_printf ("File \"%s\" is not a regular file.",
+                                         filename);
+                _cph_cups_set_internal_status (cups, error);
+                g_free (error);
+
+                close (fd);
+
+                return FALSE;
+        }
+
         /* reset the internal status: we'll use the http status */
         _cph_cups_set_internal_status (cups, NULL);
 
-        status = cupsPutFile (cups->priv->connection, resource, filename);
+        status = cupsPutFd (cups->priv->connection, resource, fd);
+
+        close (fd);
 
         _cph_cups_set_internal_status_from_http (cups, status);
 
diff --git a/src/cups.h b/src/cups.h
index 2832533..3017792 100644
--- a/src/cups.h
+++ b/src/cups.h
@@ -70,13 +70,15 @@ char *cph_cups_printer_get_uri (CphCups    *cups,
 gboolean cph_cups_is_printer_local (CphCups    *cups,
                                     const char *printer_name);
 
-gboolean cph_cups_file_get (CphCups    *cups,
-                            const char *resource,
-                            const char *filename);
-
-gboolean cph_cups_file_put (CphCups    *cups,
-                            const char *resource,
-                            const char *filename);
+gboolean cph_cups_file_get (CphCups      *cups,
+                            const char   *resource,
+                            const char   *filename,
+                            unsigned int  sender_uid);
+
+gboolean cph_cups_file_put (CphCups      *cups,
+                            const char   *resource,
+                            const char   *filename,
+                            unsigned int  sender_uid);
 
 gboolean cph_cups_server_get_settings (CphCups   *cups,
                                        GVariant **settings);