Remote DoS attack against cupsd via invalid username and malicious D-Bus library

[jsmeix]

Any user who can submit a print job to a CUPS server
can abort the cupsd running on the CUPS server by
submitting a job as a user with an invalid username.

I tested it with CUPS 1.7.5 and also the newest 2.2.5
so that I assume the issue is old and not fixed.

I think this issue here has the same root cause as
#4748
As far as I see the fix there belongs to the LPD mini daemon
but this issue here belongs to the cupsd.

How to reproduce:

On a (Linux) client system create a user with an
invalid username - I use 'binär' as username that has
the character ä (Latin small letter a with diaeresis,
i.e. the German a-umlaut) encoded in ISO-8859-1
encoding so that the username in hex byte values is

62 69 6e e4 72

Usually one cannot create such a username with "useradd"
(because it rejects invalid usernames) so that one must
manually edit /etc/passwd.

Then as this user submit a print job from the client system
to a (possibly remote) CUPS server.

The cupsd on the CUPS server will bet aborted
inside a D-Bus library call that sends SIGABRT
which aborts the whole caller process.

In /var/log/messages or nowadays in the "journalctl" output
one gets things like

2017-10-16T11:06:16.952539+02:00 e205 cupsd[14581]: process 14581: arguments to dbus_message_iter_append_basic() were incorrect, assertion "_dbus_check_is_valid_utf8 (*string_p)" failed in file dbus-message.c line 2676.
2017-10-16T11:06:16.952889+02:00 e205 cupsd[14581]: This is normally a bug in some application using the D-Bus library.
2017-10-16T11:06:16.953120+02:00 e205 cupsd[14581]: D-Bus not built with -rdynamic so unable to print a backtrace
2017-10-16T11:06:16.963025+02:00 e205 systemd[1]: cups.service: Main process exited, code=dumped, status=6/ABRT
2017-10-16T11:06:16.963303+02:00 e205 systemd[1]: cups.service: Unit entered failed state.
2017-10-16T11:06:16.963502+02:00 e205 systemd[1]: cups.service: Failed with result 'core-dump'.
2017-10-16T11:06:17.679479+02:00 e205 systemd-coredump[18461]: Process 14581 (cupsd) of user 0 dumped core.

or

Oct 16 12:57:12 caps cupsd[6930]: process 6930: arguments to dbus_message_iter_append_basic() were incorrect, assertion "_dbus_check_is_valid_utf8 (*string_p)" failed in file dbus-message.c line 2676.
Oct 16 12:57:12 caps cupsd[6930]: This is normally a bug in some application using the D-Bus library.
Oct 16 12:57:12 caps cupsd[6930]: D-Bus not built with -rdynamic so unable to print a backtrace

[jsmeix]

As expected building CUPS without DBUS support
(via configure --disable-dbus) avoids the issue.

I think in particular on remotely accessible CUPS server machines
this could be a reasonable way to avoid the issue there.

In CUPS without DBUS support there is no longer
the CUPS DBUS notifier /usr/lib/cups/notifier/dbus

I assume CUPS DBUS support is probably not needed
on dedicated CUPS server machines because DBUS
is usually used as local inter-process communication and
on CUPS server machines there are probably no processes
running that listen to DBUS messages from CUPS.

For printing on the local host via the local cupsd
the issue cannot happen when the local cupsd is not
accessible from remote because only root could manually
create local users with invalid usernames.

[michaelrsweet]

Sigh... Why do library developers insist on using abort()? Doing so prevents applications and other libraries from cleaning up after themselves, saving state, etc.

Anyways, the premise here is that you need to create a user account with an invalid name - root access required, of course - which then potentially allows a remote attacker to crash cupsd by exploiting a flaw in DBUS?

@jsmeix I need to know where in cupsd we are calling the dbus function to cause this abort to happen. The only place I see this function getting called directly is when adding the job owner to a DBUS-based notification, but that won't happen unless you have a DBUS subscription active...

[jsmeix]

I meant anyone who can become root on a remote host
that is allowed to submit a print job to a CUPS server
can do it from remote (e.g. a user with his own laptop).

We (i.e. SUSE) learned about it via a customer report
but I never got sufficient information about the customer's
environment how the invaild username on the remote host
at the customer happened to exist.

My blind guess is that the customer's remote host
is perhaps not a Linux system.

But in the end this does not matter because anyone
who can work as root on a client system can do it.

[michaelrsweet]

I still need to know where the crash is happening in cupsd to have any chance of fixing or working around this bug in DBUS (and yes, using abort() in a library is a bug IMHO).

[jsmeix]

I think next week I can try to provide details where exactly
the crash is happening in cupsd.

Below is a poposal how such kind of issues
might be avoided in general once and for all.

I fully agree it is a bug in a library when a library call
does not behave well to its caller and does not return
to its caller with an error code in case of errors but
instead misbehaves like a despotic judge plus deathsman.

On the other hand getting a signal from within a library call
is old traditional behaviour when writing anything because
then the caller could get a SIGPIPE see "man 2 write"

EPIPE
fd is connected to a pipe or socket
whose reading end is closed.
When this happens the writing process
will also receive a SIGPIPE signal.
(Thus, the write return value is seen
only if the program catches, blocks
or ignores this signal.)

How to deal with it is also documented in cups/api-filter.shtml

As far as I remember some time ago cupsd had died
because of such a SIGPIPE within an Avahi library call
that tried to output some verbose indoctrination via stderr
(which is also bad behaviour of a library call)
but there was no consumer/reader at stderr.

Because signals can happen at any time from anywhere
I think in general the caller must always protect itself
against possible unwanted signals.

Perhaps the more general solution could be
to better protect cupsd itself against unwanted signals?

Perhaps it could make sense to protect cupsd
against all signals that can be caught
(i.e. all except SIGKILL and SIGSTOP
according to "man 7 signal").

As far as I see on first glance in the CUPS 2.2.5 code
SIGABRT is not used to actually signal someting to cupsd
so that ignoring all signals that have no meaning for cupsd
(as far as I see in scheduler/main.c this are all signals
except SIGTERM, SIGHUP, and SIGCHLD)
could be the simpest and easiest way to protect
cupsd against all library calls that may result signals.

Simply put:
Why not handle SIGABRT same as SIGPIPE?

[jsmeix]

Correction regarding
"protect cupsd against all signals that can be caught":

There are some signals that indicate something went
too badly wrong to continue so that such signals
should not be ignored, first and foremost SIGSEGV
and probably also signals like SIGFPE and SIGILL.

[jsmeix]

Unbelievable!
It seems the D-Bus makers are intentionally malicious
because as far as I see in dbus-1.10.20 source code
there is in dbus/dbus-sysdeps.c

/**
 * Aborts the program with SIGABRT (dumping core).
 */
void
_dbus_abort (void)
{
  const char *s;
  
  _dbus_print_backtrace ();
  
  s = _dbus_getenv ("DBUS_BLOCK_ON_ABORT");
  if (s && *s)
    {
      /* don't use _dbus_warn here since it can _dbus_abort() */
      fprintf (stderr, "  Process %lu sleeping for gdb attach\n", _dbus_pid_for_log ());
      _dbus_sleep_milliseconds (1000 * 180);
    }
  
  abort ();
  _dbus_exit (1); /* in case someone manages to ignore SIGABRT ? */
}

It seems they even implemented double protection
against caller programs that try to deal with their evilness
first by calling abort() instead of plain raise(SIGABRT)
and then additionally by a subsequent _dbus_exit(1)
after the abort().

From my current point of view this indicates that
the D-Bus library in its current form is probably
not suitable to be called by a server process
like the cupsd that is intended to provide a
continuously available service to its users,
at least not for the cupsd on a server machine.

For home user desktop systems perhaps via
systemd socket activation that could (re)-launch
a new cupsd any time when needed such malicious
abort() calls in libraries may not hurt too much in practice.

[michaelrsweet]

@jsmeix While technically SIGABRT can be caught (and often is in debuggers), since code that calls abort() is typically not designed to function after the call it will probably have worse effects so it isn't really safe. And as you notice in this case DBUS exits anyways.

SIGPIPE is a little different since the interface is well-defined and well-used. And on some platforms there are even ways to disable SIGPIPE on a per-fd basis (which CUPS uses, when available).

The original intent of abort() is to allow a developer to stop a process under exceptional circumstances ("this should never happen") when there is no other reasonable response. Calling it when an application supplied an invalid UTF-8 string is just wrong, and the API in question should have been designed to return an error code (not just a boolean success/fail) so it is possible to discover the reason for the failure without a crash report or some other logging to a questionable file descriptor (i.e. stderr is often mapped to /dev/null in server processes).

As for whether DBUS is suitable for servers, they (the DBUS developers) market it as such and it is used widely in Linux to provide system services, so they need to fix this behavior.

I've marked this as a security issue with high priority (not critical since it requires creation of an account with an invalid username) and will continue to investigate things on my end.

[jsmeix]

@michaelrsweet
it does not require creation of an account with an invalid username.
This is only one particular way how one could get a print job
with an invalid username submitted to a cupsd.

For example with CUPS 2.2.5 on the client system
I can also abort a (possibly remote) cupsd when I use
ipptool with a specially crafted testfile like the following
print-job-invalid-user.test that I derived from
/usr/share/cups/ipptool/print-job.test

normal_user@client$ cat print-job-invalid-user.test

# Print a test page using print-job
{
        # The name of the test...
        NAME "Print file using Print-Job"
        # The operation to use
        OPERATION Print-Job
        # Attributes, starting in the operation group...
        GROUP operation-attributes-tag
        ATTR charset attributes-charset utf-8
        ATTR language attributes-natural-language en
        ATTR uri printer-uri $uri
        ATTR name requesting-user-name "binär"
        ATTR mimeMediaType document-format "text/plain"
        GROUP job-attributes-tag
        ATTR integer copies 1
        FILE "/etc/issue"
        # What statuses are OK?
        STATUS successful-ok
        STATUS successful-ok-ignored-or-substituted-attributes
        # What attributes do we expect?
        EXPECT job-id
        EXPECT job-uri
}

normal_user@client$ diff -U0 /usr/share/cups/ipptool/print-job.test print-job-invalid-user.test
--- /usr/share/cups/ipptool/print-job.test      2017-10-18 13:03:33.000000000 +0200
+++ print-job-invalid-user.test 2017-10-23 11:12:41.249751001 +0200
@@ -14,2 +14,2 @@
-       ATTR name requesting-user-name $user
-       ATTR mimeMediaType document-format $filetype
+       ATTR name requesting-user-name "binär"
+       ATTR mimeMediaType document-format "text/plain"
@@ -20 +20 @@
-       FILE $filename
+       FILE "/etc/issue"

normal_user@client$ ipptool ipp://CUPS.server/printers/queue print-job-invalid-user.test
Operation now in progress

This aborts the cupsd running on CUPS.server.

The crucial difference in print-job-invalid-user.test
is the invalid username in

    ATTR name requesting-user-name "binär"

where the ISO-8859-1 encoded username "binär"
in hex byte values is

62 69 6e e4 72

i.e. same as in my initial
#5143 (comment)

[jsmeix]

@michaelrsweet
only a side note regarding catching SIGABRT versus calling abort():

I am really not an expert in this area but according to what I read
it is rather complicated to get out of abort() because that function
does not return according to "man 3 abort".
As far as I understand it to get out of abort() one must catch
SIGABRT and then return (more precisely "jump") out of the
signal handler via longjmp(3) together with a matching setjmp(3)
to a hopefully clean prior state of memory and stack e.g. see
https://stackoverflow.com/questions/7334595/longjmp-out-of-signal-handler
and for a C++ example see
https://stackoverflow.com/questions/8934879/how-to-handle-sigabrt-signal

As far as I see this would be the only way how a caller could
safely call a library function that may abort() within the library.
But because this way it seems to be such a low level hack
I don't think libraries that deliberately abort() are meant
to be used in practice ;-)

FWIW:
There are tons of _dbus_assert calls in the dbus-1.10.20 sources
so that I assume it is in practice almost impossible to ensure
in the caller that a D-Bus library call will never abort().

[jsmeix]

I added

cupsdLogMessage(CUPSD_LOG_DEBUG2, "dbus_message_iter_append ...");

before all dbus_message_iter_append_* calls that I found.

With that I got for a job with invalid username
those last messages in /var/log/cups/error_log

D [23/Oct/2017:13:37:06 +0200] [Job 8459] Applying default options...
D [23/Oct/2017:13:37:06 +0200] add_job: requesting-user-name="binär"
I [23/Oct/2017:13:37:06 +0200] [Job 8459] Adding start banner page "none".
d [23/Oct/2017:13:37:06 +0200] copy_banner(con=0x556772d63db0[2], job=0x556772d4a220[8459], name="none")
d [23/Oct/2017:13:37:06 +0200] cupsdAddEvent(event=job-created, dest=0x556772c8f520(tofile), job=0x556772d4a220(8459), text="Job created.", ...)
d [23/Oct/2017:13:37:06 +0200] dbus_message_iter_append_string -> dbus_message_iter_append_basic subscriptions 11
d [23/Oct/2017:13:37:06 +0200] dbus_message_iter_append_uint32 -> dbus_message_iter_append_basic subscriptions 12
d [23/Oct/2017:13:37:06 +0200] dbus_message_iter_append_string -> dbus_message_iter_append_basic subscriptions 13

which matches (as expected) in my code in scheduler/subscriptions.c

  if (job)
  {
    cupsdLogMessage(CUPSD_LOG_DEBUG2, "dbus_message_iter_append_uint32 -> dbus_message_iter_append_basic subscriptions 12");
    dbus_message_iter_append_uint32(&iter, job->id);
    cupsdLogMessage(CUPSD_LOG_DEBUG2, "dbus_message_iter_append_string -> dbus_message_iter_append_basic subscriptions 13");
    dbus_message_iter_append_string(&iter, job->username);
  }

FYI:
I found 30 dbus_message_iter_append_* calls in the
CUPS 2.2.5 code and I think all might somehow lead to abort()
in the D-Bus library so that to be safe all would need to be
protected against abort() in the D-Bus library.

[michaelrsweet]

OK, I think the following change will resolve this issue:

[master 49fa498] DBUS notifications could crash the scheduler (Issue #5143)

Please let me know if you still run into problems.

[jsmeix]

With the change in scheduler/ipp.c from
49fa498
I get when I try to submit a job as a user with an
invalid username

binär@client:~> echo test | lp -h cups.server -d queue -t test
lp: Bad requesting-user-name value: "requesting-user-name": Bad name value "binär" - bad UTF-8 sequence (RFC 8011 section 5.1.3).

and when I use 'ipptool' as in
#5143 (comment)
on the CUPS server
I get

user@host:~> ipptool ipp://localhost/printers/queue print-job-invalid-user.test
Bad requesting-user-name value: "requesting-user-name": Bad name value "binär" - bad UTF-8 sequence (RFC 8011 section 5.1.3).

and on the CUPS server I get in /var/log/cups/error_log (excerpts):

d [24/Oct/2017:13:53:58 +0200] cupsdProcessIPPRequest: requesting-user-name nameWithoutLanguage 'binär'
...
D [24/Oct/2017:13:53:58 +0200] Create-Job client-error-attributes-or-values-not-supported: Bad requesting-user-name value: \"requesting-user-name\": Bad name value \"binär\" - bad UTF-8 sequence (RFC 8011 section 5.1.3).
E [24/Oct/2017:13:53:58 +0200] [Client 1] Returning IPP client-error-attributes-or-values-not-supported for Create-Job (ipp://localhost:631/printers/tofile) from 10.160.65.205

and

d [24/Oct/2017:13:55:27 +0200] cupsdProcessIPPRequest: requesting-user-name nameWithoutLanguage 'binär'
...
D [24/Oct/2017:13:55:27 +0200] Print-Job client-error-attributes-or-values-not-supported: Bad requesting-user-name value: \"requesting-user-name\": Bad name value \"binär\" - bad UTF-8 sequence (RFC 8011 section 5.1.3).
E [24/Oct/2017:13:55:27 +0200] [Client 3] Returning IPP client-error-attributes-or-values-not-supported for Print-Job (ipp://localhost:631/printers/tofile) from localhost

Accordingly from my point of view this particular issue
how the D-Bus library may abort() the cupsd is fixed.

Many thanks for the fix!

[jsmeix]

FYI:
A possibly important addedum regarding
#5143 (comment)

There I had only looked at the source code but
I had not looked at how the D-Bus makers prepared
to compile it.

The configure script in the dbus-1.10.20 sources
shows (excerpt):

$ ./configure --help | grep assert

  --enable-asserts        include assertion checks

which seems via "configure --disable-asserts" one can make
the D-Bus library without those tons of _dbus_assert calls
(that lead to the abort calls inside the D-Bus library), cf.
#5143 (comment)

This way perhaps a D-Bus library could be made
that is usable on production systems.

Also what is told on
https://www.freedesktop.org/wiki/Software/dbus/

The low-level libdbus reference implementation
...
It should be noted that the low-level implementation
is not primarily designed for application authors to use.
Rather, it is a basis for binding authors and a reference
for reimplementations. 

indicates that "just compile" the low-level libdbus
reference implementation "as is" is not meant to
result a library that can be "just used" by applications.

Accordingly it seems the root cause is not in the actual
D-Bus source code but how that source code is compiled
into a D-Bus library that is provided to the end-users.

[Z5T1]

This has been assigned CVE-2017-18248.

[michaelrsweet]

Also see Issue #5186 which applies this change more widely, to prevent similar attacks...

(both this and the referenced issue will be in the forthcoming 2.2.7 release)