Re: systemd CVE-2016-7796
Ben Hutchings <ben@decadent.org.uk> writes:
>> It looks like this patch does three things
>>
>> * It removes "assert(n > 0)".
>>
>> * It removes the now unused n parameter from the
>> manager_invoke_notify_message() function.
>>
>> * It removes the return(0) if n==0. This looks like the only relevant part.
>>
>> For the first two changes, it looks like the
>> manager_invoke_notify_message() function and hence the assert was only
>> introduced in systemd in the following commit, before tag v209. This was
>> not in the wheezy version, so I don't think these parts are required.
> [...]
>
> Right.
This means patch 4 is just the inverse of patch 3, so both become
redundant.
3. If-the-notification-message-length-is-0-ignore-the-messag.patch
4. pid1-process-zero-length-notification-messages-again.patch
Following is the debdiff of my current work, including patches 3 and
4. I also fixed the negated if condition you mentioned in a recent
email.
diff -Nru systemd-44/debian/changelog systemd-44/debian/changelog
--- systemd-44/debian/changelog 2013-10-09 21:18:41.000000000 +1100
+++ systemd-44/debian/changelog 2016-10-07 17:44:20.000000000 +1100
@@ -1,3 +1,10 @@
+systemd (44-11+deb7u5) wheezy-security; urgency=high
+
+ * Non-maintainer upload by the LTS Team.
+ * Fix CVE-2016-7796: don't return any error in manager_dispatch_notify_fd.
+
+ -- Brian May <bam@debian.org> Fri, 07 Oct 2016 17:40:21 +1100
+
systemd (44-11+deb7u4) stable-security; urgency=low
* Fix CVE-2013-4327, CVE-2013-4391 and CVE-2013-4394
diff -Nru systemd-44/debian/patches/CVE-2016-7796.patch systemd-44/debian/patches/CVE-2016-7796.patch
--- systemd-44/debian/patches/CVE-2016-7796.patch 1970-01-01 10:00:00.000000000 +1000
+++ systemd-44/debian/patches/CVE-2016-7796.patch 2016-10-07 17:30:25.000000000 +1100
@@ -0,0 +1,43 @@
+From f1e852245a30b60d5e6e0a487d049a04a40772fe Mon Sep 17 00:00:00 2001
+From: Franck Bui <fbui@suse.com>
+Date: Thu, 29 Sep 2016 11:59:49 +0200
+Subject: [PATCH] pid1: don't return any error in manager_dispatch_notify_fd()
+
+If manager_dispatch_notify_fd() fails and returns an error then the handling of
+service notifications will be disabled entirely leading to a compromised system.
+
+For example pid1 won't be able to receive the WATCHDOG messages anymore and
+will kill all services supposed to send such messages.
+---
+ src/core/manager.c | 13 +++++++++----
+ 1 file changed, 9 insertions(+), 4 deletions(-)
+
+--- a/src/manager.c
++++ b/src/manager.c
+@@ -2030,10 +2030,14 @@
+ msghdr.msg_controllen = sizeof(control);
+
+ if ((n = recvmsg(m->notify_watch.fd, &msghdr, MSG_DONTWAIT)) <= 0) {
+- if (errno == EAGAIN || errno == EINTR)
+- break;
++ if (errno != EAGAIN && errno != EINTR)
++ log_error("Failed to receive notification message: %m");
+
+- return -errno;
++ /* It's not an option to return an error here since it
++ * would disable the notification handler entirely. Services
++ * wouldn't be able to send the WATCHDOG message for
++ * example... */
++ return 0;
+ }
+
+ if (msghdr.msg_controllen < CMSG_LEN(sizeof(struct ucred)) ||
+@@ -2055,7 +2059,7 @@
+ assert((size_t) n < sizeof(buf));
+ buf[n] = 0;
+ if (!(tags = strv_split(buf, "\n\r")))
+- return -ENOMEM;
++ return 0;
+
+ log_debug("Got notification message for unit %s", u->id);
+
diff -Nru systemd-44/debian/patches/If-the-notification-message-length-is-0-ignore-the-messag.patch systemd-44/debian/patches/If-the-notification-message-length-is-0-ignore-the-messag.patch
--- systemd-44/debian/patches/If-the-notification-message-length-is-0-ignore-the-messag.patch 1970-01-01 10:00:00.000000000 +1000
+++ systemd-44/debian/patches/If-the-notification-message-length-is-0-ignore-the-messag.patch 2016-10-06 18:22:06.000000000 +1100
@@ -0,0 +1,24 @@
+From: Jorge Niedbalski <jorge.niedbalski@canonical.com>
+Date: Wed, 28 Sep 2016 18:25:50 -0300
+Subject: If the notification message length is 0, ignore the message (#4237)
+
+Fixes #4234.
+
+Signed-off-by: Jorge Niedbalski <jnr@metaklass.org>
+---
+ src/core/manager.c | 4 ++++
+ 1 file changed, 4 insertions(+)
+
+--- a/src/manager.c
++++ b/src/manager.c
+@@ -1655,6 +1655,10 @@
+
+ /* JOB_VERIFY_STARTED, JOB_RELOAD require no dependency handling */
+ }
++ if (n == 0) {
++ log_debug("Got zero-length notification message. Ignoring.");
++ return 0;
++ }
+
+ if (_ret)
+ *_ret = ret;
diff -Nru systemd-44/debian/patches/no_error_on_zero_len.patch systemd-44/debian/patches/no_error_on_zero_len.patch
--- systemd-44/debian/patches/no_error_on_zero_len.patch 1970-01-01 10:00:00.000000000 +1000
+++ systemd-44/debian/patches/no_error_on_zero_len.patch 2016-10-06 18:16:46.000000000 +1100
@@ -0,0 +1,12 @@
+--- a/src/manager.c
++++ b/src/manager.c
+@@ -2030,9 +2030,6 @@
+ msghdr.msg_controllen = sizeof(control);
+
+ if ((n = recvmsg(m->notify_watch.fd, &msghdr, MSG_DONTWAIT)) <= 0) {
+- if (n >= 0)
+- return -EIO;
+-
+ if (errno == EAGAIN || errno == EINTR)
+ break;
+
diff -Nru systemd-44/debian/patches/pid1-process-zero-length-notification-messages-again.patch systemd-44/debian/patches/pid1-process-zero-length-notification-messages-again.patch
--- systemd-44/debian/patches/pid1-process-zero-length-notification-messages-again.patch 1970-01-01 10:00:00.000000000 +1000
+++ systemd-44/debian/patches/pid1-process-zero-length-notification-messages-again.patch 2016-10-07 07:58:38.000000000 +1100
@@ -0,0 +1,26 @@
+From: =?utf-8?q?Zbigniew_J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
+Date: Thu, 29 Sep 2016 16:06:02 +0200
+Subject: pid1: process zero-length notification messages again
+
+This undoes 531ac2b234. I acked that patch without looking at the code
+carefully enough. There are two problems:
+- we want to process the fds anyway
+- in principle empty notification messages are valid, and we should
+ process them as usual, including logging using log_unit_debug().
+---
+ src/core/manager.c | 15 ++++++---------
+ 1 file changed, 6 insertions(+), 9 deletions(-)
+
+--- a/src/manager.c
++++ b/src/manager.c
+@@ -1655,10 +1655,6 @@
+
+ /* JOB_VERIFY_STARTED, JOB_RELOAD require no dependency handling */
+ }
+- if (n == 0) {
+- log_debug("Got zero-length notification message. Ignoring.");
+- return 0;
+- }
+
+ if (_ret)
+ *_ret = ret;
diff -Nru systemd-44/debian/patches/series systemd-44/debian/patches/series
--- systemd-44/debian/patches/series 2013-10-08 18:38:12.000000000 +1100
+++ systemd-44/debian/patches/series 2016-10-06 18:22:31.000000000 +1100
@@ -2,3 +2,7 @@
v44..upstream-fixes_44-11
debian-changes
secfixes
+no_error_on_zero_len.patch
+CVE-2016-7796.patch
+If-the-notification-message-length-is-0-ignore-the-messag.patch
+pid1-process-zero-length-notification-messages-again.patch
--
Brian May <bam@debian.org>
Reply to: