=== modified file 'debian/changelog' --- debian/changelog 2012-11-27 03:11:53 +0000 +++ debian/changelog 2012-11-27 03:21:03 +0000 @@ -1,3 +1,15 @@ +cups-pk-helper (0.2.1.2-1ubuntu2) quantal-security; urgency=low + + * SECURITY UPDATE: CUPS function calls were wrapped insecurely, which + could be used to upload sensitive data to a CUPS resource, or overwrite + specific files with the content of a CUPS resource. The user would have + to explicitly approve the action. (LP: #1083416) + - CVE-2012-4510 + - debian/patches/CVE-2012-4510-part1.patch: Copied from git + - debian/patches/CVE-2012-4510-part2.patch: Copied from git + + -- Jeremy Bicha Mon, 26 Nov 2012 22:12:08 -0500 + cups-pk-helper (0.2.1.2-1ubuntu1) quantal; urgency=low * Define a variable to allow access to CUPS structure fields. Fixes FTBFS. === added directory 'debian/patches' === added file 'debian/patches/CVE-2012-4510-part1.patch' --- debian/patches/CVE-2012-4510-part1.patch 1970-01-01 00:00:00 +0000 +++ debian/patches/CVE-2012-4510-part1.patch 2012-11-27 03:17:24 +0000 @@ -0,0 +1,342 @@ +From 6995d30816f0c76844dee4dc9022296a03258457 Mon Sep 17 00:00:00 2001 +From: Vincent Untz +Date: Wed, 03 Oct 2012 16:21:41 +0000 +Subject: 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. +--- +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 + #include + #include ++#include + #include + #include + #include +@@ -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); + === added file 'debian/patches/CVE-2012-4510-part2.patch' --- debian/patches/CVE-2012-4510-part2.patch 1970-01-01 00:00:00 +0000 +++ debian/patches/CVE-2012-4510-part2.patch 2012-11-27 03:17:51 +0000 @@ -0,0 +1,166 @@ +From a397b90823af3e4e697b9118022aa88f91a80989 Mon Sep 17 00:00:00 2001 +From: Vincent Untz +Date: Wed, 10 Oct 2012 07:54:18 +0000 +Subject: Also change supplementary groups when changing effective uid/gid + +Thanks to Alexander Peslyak and Sebastian Krahmer + for catching this. + +Part of fix for CVE-2012-4510. +--- +diff --git a/src/cups.c b/src/cups.c +index 6ff6cba..d45a315 100644 +--- a/src/cups.c ++++ b/src/cups.c +@@ -28,6 +28,7 @@ + #include + #include + #include ++#include + #include + #include + #include +@@ -512,31 +513,81 @@ _CPH_CUPS_IS_VALID (filename, "filename", TRUE, CPH_STR_MAXLEN) + ******************************************************/ + + static gboolean +-_cph_cups_set_effective_id (unsigned int sender_uid) ++_cph_cups_set_effective_id (unsigned int sender_uid, ++ int *saved_ngroups, ++ gid_t **saved_groups) + { + struct passwd *password_entry; ++ int ngroups; ++ gid_t *groups; ++ ++ /* avoid g_assert() because we don't want to crash here */ ++ if (saved_ngroups == NULL || saved_groups == NULL) { ++ g_critical ("Internal error: cannot save supplementary groups."); ++ return FALSE; ++ } ++ ++ *saved_ngroups = -1; ++ *saved_groups = NULL; ++ ++ ngroups = getgroups (0, NULL); ++ if (ngroups < 0) ++ return FALSE; ++ ++ groups = g_new (gid_t, ngroups); ++ if (groups == NULL && ngroups > 0) ++ return FALSE; ++ ++ if (getgroups (ngroups, groups) < 0) { ++ g_free (groups); ++ ++ return FALSE; ++ } + + password_entry = getpwuid ((uid_t) sender_uid); + + if (password_entry == NULL || +- setegid (password_entry->pw_gid) != 0) ++ setegid (password_entry->pw_gid) != 0) { ++ g_free (groups); ++ ++ return FALSE; ++ } ++ ++ if (initgroups (password_entry->pw_name, ++ password_entry->pw_gid) != 0) { ++ if (getgid () != getegid ()) ++ setegid (getgid ()); ++ ++ g_free (groups); ++ + return FALSE; ++ } ++ + + if (seteuid (sender_uid) != 0) { + if (getgid () != getegid ()) + setegid (getgid ()); + ++ setgroups (ngroups, groups); ++ g_free (groups); ++ + return FALSE; + } + ++ *saved_ngroups = ngroups; ++ *saved_groups = groups; ++ + return TRUE; + } + + static void +-_cph_cups_reset_effective_id (void) ++_cph_cups_reset_effective_id (int saved_ngroups, ++ gid_t *saved_groups) + { + seteuid (getuid ()); + setegid (getgid ()); ++ if (saved_ngroups >= 0) ++ setgroups (saved_ngroups, saved_groups); + } + + static void +@@ -1115,6 +1166,8 @@ cph_cups_file_get (CphCups *cups, + const char *filename, + unsigned int sender_uid) + { ++ int saved_ngroups = -1; ++ gid_t *saved_groups = NULL; + http_status_t status; + int fd; + struct stat file_stat; +@@ -1127,7 +1180,8 @@ cph_cups_file_get (CphCups *cups, + if (!_cph_cups_is_filename_valid (cups, filename)) + return FALSE; + +- if (!_cph_cups_set_effective_id (sender_uid)) { ++ if (!_cph_cups_set_effective_id (sender_uid, ++ &saved_ngroups, &saved_groups)) { + error = g_strdup_printf ("Cannot check if \"%s\" is " + "writable: %s", + filename, strerror (errno)); +@@ -1139,7 +1193,8 @@ cph_cups_file_get (CphCups *cups, + + fd = open (filename, O_WRONLY | O_NOFOLLOW | O_TRUNC); + +- _cph_cups_reset_effective_id (); ++ _cph_cups_reset_effective_id (saved_ngroups, saved_groups); ++ g_free (saved_groups); + + if (fd < 0) { + error = g_strdup_printf ("Cannot open \"%s\": %s", +@@ -1200,6 +1255,8 @@ cph_cups_file_put (CphCups *cups, + const char *filename, + unsigned int sender_uid) + { ++ int saved_ngroups = -1; ++ gid_t *saved_groups = NULL; + http_status_t status; + int fd; + struct stat file_stat; +@@ -1212,7 +1269,8 @@ cph_cups_file_put (CphCups *cups, + if (!_cph_cups_is_filename_valid (cups, filename)) + return FALSE; + +- if (!_cph_cups_set_effective_id (sender_uid)) { ++ if (!_cph_cups_set_effective_id (sender_uid, ++ &saved_ngroups, &saved_groups)) { + error = g_strdup_printf ("Cannot check if \"%s\" is " + "readable: %s", + filename, strerror (errno)); +@@ -1224,7 +1282,8 @@ cph_cups_file_put (CphCups *cups, + + fd = open (filename, O_RDONLY); + +- _cph_cups_reset_effective_id (); ++ _cph_cups_reset_effective_id (saved_ngroups, saved_groups); ++ g_free (saved_groups); + + if (fd < 0) { + error = g_strdup_printf ("Cannot open \"%s\": %s", + === added file 'debian/patches/series' --- debian/patches/series 1970-01-01 00:00:00 +0000 +++ debian/patches/series 2012-11-27 03:18:34 +0000 @@ -0,0 +1,2 @@ +CVE-2012-4510-part1.patch +CVE-2012-4510-part2.patch