[Pkg-cups-devel] Bug#692791: Bug#692791: Bug#692791: members of lpadmin can read every file on server via cups

[Jeff Licquia]

[Re-adding security team to CC.]

On 11/10/2012 07:44 AM, Martin Pitt wrote:
> Didier 'OdyX' Raboud [2012-11-10 12:48 +0100]:
>> * Have cupsd run as lp user
> 
> We had done that in Debian for several years for security reasons. We
> had a huge patch to make most of cups work as user "lp", but at some
> point I gave up: it caused too many bugs, didn't work with a lot of
> third-party drivers, and broke with every new upstream release.
> Upstream has never bought into the idea of running the main server as
> an unprivileged system user unfortunately.
> 
> So this is possible in principle, but will mean a huge maintenance
> overhead.

Maybe this situation will help make the case. :-)

>> * Forbid any changes to the config file from the webinterface
> 
> That would drop a huge piece of functionality.

CUPS allows changes to the config file in two ways: changing a small
subset of settings in a way that's checked server-side, and editing
cupsd.conf in a browser by downloading the file, and then uploading the
edited version post-edit.  The latter functionality is what's being
exploited here, and it strikes me as far more dangerous than the former.

I've attached a proposed dpatch which disables just cupsd.conf editing.
 It's against the squeeze version, but applies fairly cleanly to the
wheezy version.  I've tested it against the exploit script, which no
longer dumps /etc/shadow to stdout with the patch applied.

Best of all, this patch doesn't completely disable configuring the
server from a browser, just the wholesale cupsd.conf edit.

I'm not sure this is "upstream-worthy", but it might do until upstream
gets a better fix written.

>> * Another idea ?
> 
> cupsd could temporarily drop privileges to lp when reading log files;
> with that you are restricted to reading world-readable files as well
> as cups' own files, which should be fine?

I suspect this fix would end up in a game of "whack-a-mole", as we find
interesting settings which trick cupsd into doing nefarious things.


-------------- next part --------------
#! /bin/sh /usr/share/dpatch/dpatch-run
## bug-692791.dpatch by Jeff Licquia <licquia at debian.org>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: Disable editing of cupsd.conf via web interface.

@DPATCH@
diff -urNad '--exclude=CVS' '--exclude=.svn' '--exclude=.git' '--exclude=.arch' '--exclude=.hg' '--exclude=_darcs' '--exclude=.bzr' cups-1.4.4~/cgi-bin/admin.c cups-1.4.4/cgi-bin/admin.c
--- cups-1.4.4~/cgi-bin/admin.c	2012-11-10 16:12:08.000000000 -0500
+++ cups-1.4.4/cgi-bin/admin.c	2012-11-10 16:20:37.000000000 -0500
@@ -1881,6 +1881,24 @@
   else if (cgiGetVariable("SAVECHANGES") && cgiGetVariable("CUPSDCONF"))
   {
    /*
+    * XXX: Disable web editing of config file for security reasons.
+    *      This is a quick-n-dirty hack to just turn the functionality
+    *      off temporarily until upstream/someone has time to fix the
+    *      problem properly.  See Debian bug 692791 for details.
+    */
+
+    cgiStartHTML(cgiText(_("Edit Configuration File")));
+    cgiSetVariable("MESSAGE", cgiText(_("Unable to upload cupsd.conf file:")));
+    cgiSetVariable("ERROR", strerror(EPERM));
+    cgiCopyTemplateLang("error.tmpl");
+    cgiEndHTML();
+    return;
+
+   /*
+    * End of quick security hack.  Original implementation follows.
+    */
+
+   /*
     * Save hand-edited config file...
     */